[LOADED_DOCS: NONE]
Update protocol, docs-discovery skill, and doc layering - Switched AI AGENT CORE PROTOCOL to new `[LOADED_DOCS: N files (+K this turn: ...)]` prefix format in all primary instruction files; updated examples and enforcement details. - Added `docs-discovery` skill for proactive .md doc loading before code search; documented usage and integration as a shared agent skill. - Introduced `## Protocol History` section and `LLM_PROTOCOL_DECISIONS.md` decision log for cross-repo protocol change tracking. - Expanded protocol-audit skill and `REPOS.md` to support 8 instruction files (primary/inherit split), new invariants, and known issues. - Added/updated structured TODO and issues docs for serialization, logging, and SignalR binary protocol. - Improved cross-references, doc layering, and clarified documentation-first coding workflow. - Various minor doc clarifications and formatting for protocol consistency.
This commit is contained in:
parent
06a9efd7f9
commit
61509f1b95
|
|
@ -0,0 +1,79 @@
|
|||
# LLM Protocol Decisions
|
||||
|
||||
Append-only log of decisions about the AI AGENT CORE PROTOCOL and related LLM tooling (skills, instruction files, shared conventions) across the 8 known `.github/copilot-instructions.md` files.
|
||||
|
||||
## Purpose
|
||||
|
||||
Quick lookup for **why** a rule is the way it is, to avoid:
|
||||
- Re-debating decisions that were already discussed and resolved
|
||||
- Silently "optimizing away" a rule whose edge-case purpose is no longer obvious
|
||||
- Losing context between sessions / across team members
|
||||
|
||||
## Scope
|
||||
|
||||
**Tracked here:**
|
||||
- `.github/copilot-instructions.md` rule additions / removals / rewordings
|
||||
- `CLAUDE.md` changes
|
||||
- New, modified, or removed agent skills (`docs-discovery`, `protocol-audit`, etc.)
|
||||
- Protocol-level structural shifts (doc layout, file location conventions)
|
||||
|
||||
**NOT tracked here:**
|
||||
- Code refactors or feature work — use commit messages / PR descriptions
|
||||
- Code-architecture decisions — see `docs/ARCHITECTURE.md` at the relevant layer
|
||||
- Trivial fixes (typos, path corrections without rule-semantic change)
|
||||
|
||||
## Writing rules
|
||||
|
||||
1. **Append-only.** If a decision is reversed or superseded, add a NEW entry that points to the old one. Do not rewrite history — the evolution itself is informative.
|
||||
2. **One row per decision.** Keep the rationale concise; the purpose is scan-ability, not prose.
|
||||
3. **`Affected` column** uses the scope codes defined below — always be explicit about which files were touched.
|
||||
4. **Explicit consent required for additions** (per Rule #5 of the active `copilot-instructions.md`). The LLM proposes the entry; the user reviews and approves before the write.
|
||||
5. **Reference this log** before proposing a protocol change — if the change was already discussed, the existing entry may save a debate.
|
||||
|
||||
## Scope codes (for the `Affected` column)
|
||||
|
||||
| Code | Meaning |
|
||||
|------------------------|-----------------------------------------------------------------------------------------------------------|
|
||||
| `5× primary` | All 5 primary `copilot-instructions.md` (AyCode.Core, AyCode.Blazor, Libraries, FruitBank, FruitBankHybridApp) |
|
||||
| `4× non-Core primary` | The 4 primary files except AyCode.Core |
|
||||
| `3× inherit` | The 3 inherit files (Mango.Nop.Core, Nop.Plugin.Misc.AIPlugin, Mango.FruitBank) |
|
||||
| `all 8` | All 8 files: 5 primary + 3 inherit |
|
||||
| `<repo>/...` | A single specific file path |
|
||||
| `<skill-name>/...` | Skill directory under `.github/skills/` |
|
||||
|
||||
The "primary" vs "inherit" distinction is defined in `protocol-audit/references/REPOS.md`.
|
||||
|
||||
## 2026
|
||||
|
||||
| Date | Decision | Rationale | Affected |
|
||||
|------------|-------------------------------------------------------|---------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------|
|
||||
| 2026-04-20 | Added CROSS-REPO HARD-GATE to Rule #2 | Docs-before-code was applied only to the own repo, not external deps | `5× primary` |
|
||||
| 2026-04-20 | Added PER-QUESTION DOC-FIRST to Rule #2 | LLM skipped checking for relevant `.md` before jumping to code search | `5× primary` |
|
||||
| 2026-04-21 | Expanded Rule #5 scope: "code/files" → "any file..." | Memory / config / docs edits also need consent; old wording too narrow | `5× primary` |
|
||||
| 2026-04-22 | Added Auto-detection triggers to Rule #4 | "realize" was subjective; post-compaction reset to `[LOADED_DOCS: NONE]` didn't fire reliably | `5× primary` |
|
||||
| 2026-04-22 | Added "in context" definition to Rule #3 | Summary content was mistaken for actually-loaded docs (lossy compression) | `5× primary` |
|
||||
| 2026-04-22 | Unified AyCode.Blazor Rule #3 with the other 4 repos | Was custom "CRITICAL TOOL EXECUTION FIREWALL" variant; normalized | `AyCode.Blazor/copilot-instructions.md` |
|
||||
| 2026-04-22 | "strictly maintain rule X" → "rule 3" | Per-repo numbers varied (15/18/19/20/21); unified reference | `5× primary` |
|
||||
| 2026-04-22 | Created `protocol-audit` skill at repo-level | Cross-repo consistency check; `.github/skills/` chosen over personal paths (DRY) | `protocol-audit/SKILL.md` + `protocol-audit/references/REPOS.md` |
|
||||
| 2026-04-22 | Added `## Shared Agent Skills` section (protocol-audit) | Discoverability of AyCode.Core-hosted skills from dependent repos | `4× non-Core primary` |
|
||||
| 2026-04-23 | Created `docs-discovery` skill (parallel session) | Paired-doc (`main` + `_ISSUES` + `_TODO`) auto-loading before code search | `docs-discovery/SKILL.md` + `AyCode.Core/copilot-instructions.md` |
|
||||
| 2026-04-23 | Extended `## Shared Agent Skills` with `docs-discovery` (parallel session) | Both skills now listed in each dependent repo | `4× non-Core primary` + `Mango.Nop.Core/copilot-instructions.md` |
|
||||
| 2026-04-24 | LOADED_DOCS prefix: full list → count+delta format | Long lists became visual noise at scale; delta preserves self-commitment | `5× primary` + `docs-discovery/SKILL.md` |
|
||||
| 2026-04-24 | Created this Decision Log | Institutional memory for protocol evolution; avoid re-debating resolved choices | `AyCode.Core/.github/LLM_PROTOCOL_DECISIONS.md` |
|
||||
| 2026-04-24 | Added `## Protocol History` section | Cross-repo discoverability of the Decision Log | `4× non-Core primary` + `Mango.Nop.Core/copilot-instructions.md` |
|
||||
| 2026-04-24 | Filled empty Nop.Plugin.Misc.AIPlugin instruction file | Previously 0 bytes; now minimal inherit-pattern referencing AyCode.Core + both skills + log | `Nop.Plugin.Misc.AIPlugin/copilot-instructions.md` |
|
||||
| 2026-04-24 | Filled empty Mango\FruitBank instruction file | Previously 0 bytes; now minimal inherit-pattern with ⚠️ "purpose TBD" notice | `Mango/FruitBank/copilot-instructions.md` |
|
||||
| 2026-04-24 | Expanded REPOS.md: 5 repos → 8 (primary + inherit) | Audit scope was incomplete; sub-project & plugin files were missed | `protocol-audit/references/REPOS.md` |
|
||||
| 2026-04-24 | Fixed Mango.Nop.Core `@repo` path (7 `..` → 6 `..`) | Previous path resolved to `H:\Aycode\...` (non-existent). Corrected relative depth from repo root | `Mango.Nop.Core/copilot-instructions.md` |
|
||||
| 2026-04-24 | Resolved Mango.FruitBank purpose (nopCommerce host) | Confirmed: directory is a nopCommerce deployment for FruitBank company, hosting Nop.Plugin.Misc.AIPlugin. Layer 4 (host), not TBD. Updated content accordingly. | `Mango/FruitBank/copilot-instructions.md` |
|
||||
| 2026-04-24 | `protocol-audit` v1.0 → v2.0: primary/inherit invariant split | SKILL.md now applies Common + Primary invariants to rows 1-5, Common + Inherit + Cross-cutting to rows 6-8. New invariants: P3 (Rule #1 count+delta format), X1 (Shared Agent Skills), X2 (Protocol History). | `protocol-audit/SKILL.md` + `protocol-audit/references/REPOS.md` (issues cleared) |
|
||||
|
||||
## Known follow-ups
|
||||
|
||||
*(No open follow-ups. All items from previous audits resolved — see dated entries above for fix history.)*
|
||||
|
||||
## Notes
|
||||
|
||||
- Dates before 2026-04-24 are retroactive reconstructions from session history; intra-day ordering is approximate.
|
||||
- For precise attribution of individual file changes, consult the git history of the affected files.
|
||||
- This log is a **summary** artifact — it records the decision and its rationale, not the full diff.
|
||||
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
|
|
@ -1,25 +1,75 @@
|
|||
# Repos under protocol-audit
|
||||
|
||||
Each row is one repo whose `.github/copilot-instructions.md` must obey the shared AI AGENT CORE PROTOCOL.
|
||||
Each row is one `.github/copilot-instructions.md` file that participates in the shared AI AGENT CORE PROTOCOL — either as **Primary** (full numbered-rule protocol) or **Inherit** (references AyCode.Core's protocol, minimal own content).
|
||||
|
||||
## Primary protocol files
|
||||
|
||||
These files contain the full numbered AI AGENT CORE PROTOCOL (Rules #1-5), `@repo` block, and Conventions. All structural, rule-content, and cross-reference invariants apply.
|
||||
|
||||
| # | Name | Absolute path | Layer |
|
||||
|---|---------------------|----------------------------------------------------------------------------|------------------|
|
||||
| 1 | AyCode.Core | `H:\Applications\Aycode\Source\AyCode.Core` | framework |
|
||||
| 2 | AyCode.Blazor | `H:\Applications\Aycode\Source\AyCode.Blazor` | framework |
|
||||
| 3 | Libraries | `H:\Applications\Mango\Source\NopCommerce.Common\4.70\Libraries` | shared libraries |
|
||||
| 4 | FruitBank | `H:\Applications\Mango\Source\FruitBank` | product |
|
||||
| 5 | FruitBankHybridApp | `H:\Applications\Mango\Source\FruitBankHybridApp` | product |
|
||||
| 1 | AyCode.Core | `H:\Applications\Aycode\Source\AyCode.Core` | framework (0) |
|
||||
| 2 | AyCode.Blazor | `H:\Applications\Aycode\Source\AyCode.Blazor` | framework (1) |
|
||||
| 3 | Libraries | `H:\Applications\Mango\Source\NopCommerce.Common\4.70\Libraries` | shared libs (2) |
|
||||
| 4 | FruitBank | `H:\Applications\Mango\Source\FruitBank` | product (3) |
|
||||
| 5 | FruitBankHybridApp | `H:\Applications\Mango\Source\FruitBankHybridApp` | product (3) |
|
||||
|
||||
## Inherit protocol files
|
||||
|
||||
These files reference AyCode.Core's protocol via blockquote and do NOT repeat the numbered rules. Only the reduced invariant set applies (see "Invariants by type" below).
|
||||
|
||||
| # | Name | Absolute path | Layer | Role |
|
||||
|---|----------------------------|------------------------------------------------------------------------------------------------|------------------|-------------------------------------------|
|
||||
| 6 | Mango.Nop.Core | `H:\Applications\Mango\Source\NopCommerce.Common\4.70\Libraries\Mango.Nop.Core` | framework (2) | Domain framework for NopCommerce plugins |
|
||||
| 7 | Nop.Plugin.Misc.AIPlugin | `H:\Applications\Mango\Source\NopCommerce.Common\4.70\Plugins\Nop.Plugin.Misc.AIPlugin` | consumer (3) | FruitBank plugin source code |
|
||||
| 8 | Mango.FruitBank | `H:\Applications\Mango\FruitBank` | host (4) | nopCommerce deployment for FruitBank co. |
|
||||
|
||||
The instruction file to audit for each is: `<abs-path>\.github\copilot-instructions.md`.
|
||||
|
||||
## Expected own-dep-repos (for @repo block path validation)
|
||||
|
||||
| Repo | Expected deps (relative path from this repo's root) |
|
||||
|--------------------|-------------------------------------------------------------------------------------------------------------|
|
||||
| AyCode.Core | — (none) |
|
||||
| AyCode.Blazor | `AyCode.Core: ../AyCode.Core` |
|
||||
| Libraries | `AyCode.Core: ../../../../../Aycode/Source/AyCode.Core` |
|
||||
| FruitBank | `AyCode.Core: ../../../Aycode/Source/AyCode.Core`, `Libraries: ../NopCommerce.Common/4.70/Libraries` |
|
||||
### Primary
|
||||
|
||||
| Repo | Expected deps (relative path from this repo's root) |
|
||||
|--------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------|
|
||||
| AyCode.Core | — (none) |
|
||||
| AyCode.Blazor | `AyCode.Core: ../AyCode.Core` |
|
||||
| Libraries | `AyCode.Core: ../../../../../Aycode/Source/AyCode.Core` |
|
||||
| FruitBank | `AyCode.Core: ../../../Aycode/Source/AyCode.Core`, `Libraries: ../NopCommerce.Common/4.70/Libraries` |
|
||||
| FruitBankHybridApp | `AyCode.Core: ../../../Aycode/Source/AyCode.Core`, `AyCode.Blazor: ../../../Aycode/Source/AyCode.Blazor`, `Libraries: ../NopCommerce.Common/4.70/Libraries` |
|
||||
|
||||
When modifying the repo set (add/remove/move), update this table. SKILL.md remains stable.
|
||||
### Inherit
|
||||
|
||||
| Repo | Expected deps (relative path from this repo's root) |
|
||||
|---------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------|
|
||||
| Mango.Nop.Core | `AyCode.Core: ../../../../../../Aycode/Source/AyCode.Core` |
|
||||
| Nop.Plugin.Misc.AIPlugin | `Mango.Nop.Core: ../../Libraries/Mango.Nop.Core`, `AyCode.Core: ../../../../../../Aycode/Source/AyCode.Core` |
|
||||
| Mango.FruitBank | `AyCode.Core: ../../Aycode/Source/AyCode.Core` |
|
||||
|
||||
## Invariants by type
|
||||
|
||||
**Primary files (1-5)** — full invariant set per `SKILL.md`:
|
||||
- Rule numbering contiguous 1..N
|
||||
- Rule #2 contains `CROSS-REPO HARD-GATE` and `PER-QUESTION DOC-FIRST`
|
||||
- Rule #3 is `STRICT NO-RE-READ POLICY (ANTI-LOOP)` and contains "in context" definition (`lossy compressions`)
|
||||
- Rule #4 contains auto-detection triggers
|
||||
- Rule #5 contains broad scope wording (`any file (code, documentation, configuration, memory, or otherwise)`)
|
||||
- `strictly maintain rule 3` reference exists
|
||||
- `## Shared Agent Skills` section with both skills listed
|
||||
- `## Protocol History` section referencing `AyCode.Core/.github/LLM_PROTOCOL_DECISIONS.md`
|
||||
- `@repo` block has all 4 required fields, paths resolve to existing directories
|
||||
|
||||
**Inherit files (6-8)** — reduced invariant set:
|
||||
- References AyCode.Core's protocol via substring: `follows the AI Agent Core Protocol defined in AyCode.Core`
|
||||
- Has `## Shared Agent Skills` section listing **both** skills (docs-discovery + protocol-audit)
|
||||
- Has `## Protocol History` section referencing `AyCode.Core/.github/LLM_PROTOCOL_DECISIONS.md`
|
||||
- `@repo` block (if present) has all 4 required fields, paths resolve to existing directories
|
||||
- Numbered rules are NOT required (they are inherited from AyCode.Core)
|
||||
|
||||
## Known issues
|
||||
|
||||
*(No open issues. Previously tracked — Mango.Nop.Core path off-by-one and Mango.FruitBank purpose TBD — are resolved; see `AyCode.Core/.github/LLM_PROTOCOL_DECISIONS.md` for the fix history.)*
|
||||
|
||||
## Maintenance note
|
||||
|
||||
When modifying the repo set (add, remove, or relocate), update this table. `SKILL.md` remains stable because it reads this file at runtime.
|
||||
|
|
|
|||
|
|
@ -83,3 +83,51 @@ Same constraint as SER-2 — `IBinaryInputBase` interface is synchronous. `ReadA
|
|||
**Affects:** Generated reader code
|
||||
|
||||
The source generator emits `null` assignments for non-nullable reference type properties during deserialization (before the value is read from the stream). This produces CS8625 warnings. Functionally harmless — the property is always assigned before use.
|
||||
|
||||
## Buffer Writer (BWO)
|
||||
|
||||
### BWO-1: Struct copy semantics
|
||||
|
||||
**Status:** By design
|
||||
**Affects:** `BufferWriterBinaryOutput` value-type assignment
|
||||
|
||||
Assigning a `BufferWriterBinaryOutput` value creates an independent copy. State changes (e.g. `_committedBytes` via `Grow`/`Flush`) are not reflected in the original. Copy back after use if needed.
|
||||
|
||||
### BWO-2: Initialize resets tracking
|
||||
|
||||
**Status:** By design
|
||||
**Affects:** `BufferWriterBinaryOutput.Initialize` (context mode)
|
||||
|
||||
`Initialize` sets `_committedBytes = 0`. Standalone bytes written before are lost if the BWO is then passed to a context. Call `FlushAndReset()` first, or track standalone bytes separately.
|
||||
|
||||
### BWO-3: Constructor acquires chunk
|
||||
|
||||
**Status:** Acceptable (not a leak)
|
||||
**Affects:** `BufferWriterBinaryOutput` ctor
|
||||
|
||||
`AcquireChunk` runs in ctor for standalone readiness. Redundant if only context mode is used (context `Initialize` acquires its own). Not a leak — consecutive `GetMemory` without `Advance` returns overlapping memory.
|
||||
|
||||
### BWO-4: No mode mixing
|
||||
|
||||
**Status:** By design
|
||||
**Affects:** `BufferWriterBinaryOutput` — context vs standalone mode
|
||||
|
||||
A single instance must not use context + standalone modes simultaneously — buffer states desynchronize. One mode per lifecycle phase; `FlushAndReset()` as boundary between modes.
|
||||
|
||||
### SGEN-2: Consumer entity with `new` Id shadowing — excluded from SGen
|
||||
|
||||
**Status:** Workaround-in-place (compiled-expression fallback)
|
||||
**Affects:** Any consumer entity whose base class hides `BaseEntity.Id` with `readonly new int Id { get; }` pattern (e.g. `DiscountProductMapping` in Mango.Nop.Core)
|
||||
|
||||
When the base class shadows `Id` with a setter-less `new int Id { get; }`, SGen can't emit a setter without CS0200. Runtime falls back to compiled-expression serialization for these types. Low priority — affects a small number of consumer entities.
|
||||
|
||||
**Related TODO:** `BINARY_TODO.md#todo-02`
|
||||
|
||||
## Cross-cutting (also tracked in SignalR-side docs)
|
||||
|
||||
### XCUT-1: JSON-in-Binary request parameters
|
||||
|
||||
**Status:** Major tech debt, planned replacement (coordinated)
|
||||
**Affects:** SignalR client→server request parameter path
|
||||
|
||||
Request parameters currently use JSON inside a Binary envelope (`SignalPostJsonDataMessage<T>`). Response path already uses pure Binary. Planned migration is a cross-project coordinated change — see `AyCode.Services/docs/SIGNALR_ISSUES.md` PROTO section and `BINARY_TODO.md#todo-01` for the broader picture. Do NOT attempt as a side-effect of unrelated work.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,18 @@
|
|||
# AcBinarySerializer — TODO
|
||||
|
||||
## Priority legend
|
||||
- **P0** blocker · **P1** important · **P2** nice-to-have · **P3** idea
|
||||
|
||||
---
|
||||
|
||||
## TODO-01: Replace JSON-in-Binary request parameters
|
||||
**Priority:** P1 · **Type:** Refactor · **Related:** `BINARY_ISSUES.md#issue-01`, `AyCode.Services/docs/SIGNALR_TODO.md`
|
||||
|
||||
Migrate client→server request parameters from JSON-in-Binary envelope to direct Binary serialization (matching response path). Coordinated change across client, server, and all consuming projects. Do NOT attempt as side-effect of unrelated work.
|
||||
|
||||
**Acceptance:** `SignalPostJsonDataMessage<T>` replaced by a `SignalPostBinaryDataMessage<T>` (or equivalent); no JSON round-trip on the wire for request params; benchmarks confirm no regression.
|
||||
|
||||
## TODO-02: Re-evaluate DiscountProductMapping SGen exclusion
|
||||
**Priority:** P3 · **Type:** Investigation · **Related:** `BINARY_ISSUES.md#issue-02`
|
||||
|
||||
Investigate whether the `new int Id` shadowing pattern can be handled by SGen (via base-class introspection, property-setter lookup on the base) to eliminate the runtime compiled-expression fallback for this entity class.
|
||||
|
|
@ -61,10 +61,7 @@ Context/standalone share only `IBufferWriter` ref and `_committedBytes`.
|
|||
|
||||
### Known Limitations
|
||||
|
||||
1. **Struct copy semantics:** value type → assignment creates independent copy. State changes (e.g. `_committedBytes` via `Grow`/`Flush`) not reflected in original. Copy back after use if needed.
|
||||
2. **Initialize resets tracking:** `_committedBytes = 0`. Standalone bytes lost if BWO then passed to context. Use `FlushAndReset()` before, track standalone bytes separately.
|
||||
3. **Constructor acquires chunk:** `AcquireChunk` in ctor for standalone readiness. Redundant if only context mode used (context `Initialize` acquires its own). Not a leak — consecutive `GetMemory` without `Advance` returns overlapping memory.
|
||||
4. **No mode mixing:** single instance must not use context+standalone simultaneously. Buffer states desynchronize. One mode per lifecycle phase, `FlushAndReset()` as boundary.
|
||||
Buffer-writer contract limitations are tracked in `BINARY_ISSUES.md` under the **Buffer Writer (BWO)** category — struct copy semantics, init-reset tracking, ctor chunk acquisition, no-mode-mixing rule.
|
||||
|
||||
### Chunk Size
|
||||
|
||||
|
|
|
|||
|
|
@ -118,6 +118,89 @@ Each writer's constructor signature must accept `(AppType, LogLevel, string?)`.
|
|||
|
||||
`AcLogWriterBase` also reads its own `LogLevel` from config by matching its `AssemblyQualifiedName` against the `LogWriterType` entries. This means a writer instantiated manually (not via config) can still pick up config values if a matching entry exists.
|
||||
|
||||
## DI-Based Factory Pattern
|
||||
|
||||
Modern, framework-first alternative to the config-reading `AcLoggerBase(string)` ctor. No runtime reflection over writer config; concrete writer types resolved from DI. Recommended for all modern projects (MAUI, WASM, ASP.NET Core) — the config-reading path is filesystem-bound and unsuitable for MAUI/WASM (see `LOGGING_ISSUES.md#issue-02`).
|
||||
|
||||
### Consumer setup in Program.cs
|
||||
|
||||
```csharp
|
||||
// 1. Bind options from appsettings.json
|
||||
services.Configure<AcLoggerOptions>(configuration.GetSection("AyCode:Logger"));
|
||||
|
||||
// 2. Register writer(s) as DI singletons
|
||||
services.AddSingleton<IAcLogWriterBase, MyConsoleWriter>();
|
||||
// Or, for client-scoped marker:
|
||||
// services.AddSingleton<IAcLogWriterClientBase, MyRemoteWriter>();
|
||||
|
||||
// 3. Register the logger factory — registers Func<string, TLogger> singleton in DI
|
||||
services.AddAcLoggerFactory<MyConcreteLogger>();
|
||||
// Or, with custom writer marker (pulls only TWriterBase-registered writers):
|
||||
// services.AddAcLoggerFactory<MyConcreteLogger, IAcLogWriterClientBase>();
|
||||
```
|
||||
|
||||
Consumers inject `Func<string, MyConcreteLogger>` and invoke it with a category name to obtain category-scoped logger instances.
|
||||
|
||||
### Appsettings.json shape
|
||||
|
||||
```json
|
||||
{
|
||||
"AyCode": {
|
||||
"Logger": {
|
||||
"AppType": "Web",
|
||||
"LogLevel": "Debug"
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Only `AppType` and `LogLevel` bind to `AcLoggerOptions`. The legacy `LogWriters[]` array used by the config-reading ctor is independent and unused in this pattern.
|
||||
|
||||
### AcLoggerOptions
|
||||
|
||||
| Property | Type | Default | Purpose |
|
||||
|----------|------|---------|---------|
|
||||
| `AppType` | `AppType` | `Server` | Stamped on each log entry |
|
||||
| `LogLevel` | `LogLevel` | `Info` | Global minimum level (Logger's own gate) |
|
||||
|
||||
Extend the POCO as needed; consumer's `services.Configure<AcLoggerOptions>` binding picks up new properties automatically.
|
||||
|
||||
### TLogger ctor requirement
|
||||
|
||||
`AddAcLoggerFactory<TLogger>` uses `Activator.CreateInstance(typeof(TLogger), AppType, LogLevel, categoryName, writers)`. `TLogger` must expose a public constructor with signature:
|
||||
```csharp
|
||||
public TLogger(AppType appType, LogLevel logLevel, string? categoryName, params IAcLogWriterBase[] logWriters)
|
||||
```
|
||||
|
||||
If your concrete logger doesn't have this signature, register a manual factory instead:
|
||||
```csharp
|
||||
services.AddSingleton<Func<string, MyConcreteLogger>>(sp => category => new MyConcreteLogger(...));
|
||||
```
|
||||
|
||||
### Writer-marker scoping (two overloads)
|
||||
|
||||
- `AddAcLoggerFactory<TLogger>()` — all `IAcLogWriterBase`-registered writers go into the logger
|
||||
- `AddAcLoggerFactory<TLogger, TWriterBase>()` — only writers registered as `TWriterBase` (e.g. `IAcLogWriterClientBase` for client-only)
|
||||
|
||||
Use the two-arg overload when the consumer separates client-only and server-only writers via a marker interface.
|
||||
|
||||
### Companion extension: AddAcDefaults (SignalR)
|
||||
|
||||
For SignalR client setup that wires the same `AcLoggerBase` instance into Microsoft.Extensions.Logging, use `AcSignalRConnectionExtensions.AddAcDefaults(builder, logger, connectionOptions)` — it bundles `AddAcConnection` + `ConfigureLogging(AddAcLogger)` in a single call. See `AyCode.Services/docs/SIGNALR.md`.
|
||||
|
||||
### Comparison: config-reading ctor vs DI-based factory
|
||||
|
||||
| Aspect | Config-reading ctor | DI-based factory |
|
||||
|--------|---------------------|------------------|
|
||||
| Writer instantiation | `Activator.CreateInstance` per writer entry | Resolved from DI singletons |
|
||||
| Writer config location | `AyCode:Logger:LogWriters[]` | `services.AddSingleton<IAcLogWriterBase, ...>()` |
|
||||
| Writer ctor requirement | `(AppType, LogLevel, string?)` | Consumer-defined (via DI) |
|
||||
| Writer lifecycle | Per-logger instance | Singleton, shared across loggers |
|
||||
| Typical use | Legacy / server-side config-driven | Modern DI: MAUI, WASM, ASP.NET Core |
|
||||
| MAUI/WASM-safe | ❌ (filesystem-bound `AcEnv.AppConfiguration`) | ✅ |
|
||||
|
||||
Both patterns can coexist — consumer picks per scenario. Related issues: `LOGGING_ISSUES.md`. Planned unification: `LOGGING_TODO.md#todo-03`.
|
||||
|
||||
## Core Components
|
||||
|
||||
### AcLoggerBase
|
||||
|
|
|
|||
|
|
@ -0,0 +1,155 @@
|
|||
# Logger — Known Issues
|
||||
|
||||
For planned/actionable work see `LOGGING_TODO.md`.
|
||||
|
||||
## ISSUE-01: NopLogWriter ctor signature mismatch (consumer-specific but framework-exposed)
|
||||
|
||||
**Severity:** Minor (caught, non-blocking, but noisy) · **Status:** Open · **Area:** Writer-instantiation contract (`AcLoggerBase(string)` config-reading ctor)
|
||||
|
||||
### Description
|
||||
When `AcLoggerBase(string? categoryName)` (config-reading ctor) runs, it iterates `AyCode:Logger:LogWriters[]` and calls `Activator.CreateInstance(writerType, AppType, LogLevel, categoryName)`. Some consumer writers (notably a NopCommerce-plugin-level `NopLogWriter`) have DI-injected ctors instead (e.g. `(INopLoggerMsSqlNopDataProvider, CommonSettings, CustomerSettings, IWebHelper, ILogger, string?)`), so no ctor matches → `MissingMethodException`.
|
||||
|
||||
Exception is caught in `AcLoggerBase` try/catch, logged to `Console.Error`, loop continues with other writers. Non-blocking — protocol construction succeeds — but produces noisy startup logs per `Logger` instance.
|
||||
|
||||
### Root cause
|
||||
Two logger-construction paradigms coexist:
|
||||
- Config-reading ctor expects `(AppType, LogLevel, string?)` writer ctors for `Activator` convenience
|
||||
- DI-injected writers have arbitrary ctor signatures
|
||||
|
||||
### Known workaround
|
||||
Console.Error noise tolerated. Alternatively, consumer uses DI-based `AddAcLoggerFactory<TLogger>` (see LOGGING.md) instead of the config-reading ctor — this path doesn't touch `LogWriters[]`.
|
||||
|
||||
### Related TODO
|
||||
`LOGGING_TODO.md#todo-01`
|
||||
|
||||
## ISSUE-02: AcEnv.AppConfiguration is filesystem-bound, MAUI/WASM-unsafe
|
||||
|
||||
**Severity:** Minor · **Status:** Documented limitation · **Area:** `AyCode.Core.Consts.AcEnv`
|
||||
|
||||
### Description
|
||||
`AcEnv.AppConfiguration` is a static lazy singleton calling `new ConfigurationBuilder().AddJsonFile("appsettings.json").Build()` on first access. Reads current-working-directory filesystem. Throws on MAUI (no physical appsettings next to exe) and WASM (no filesystem at all).
|
||||
|
||||
### Root cause
|
||||
Design predates `IOptions`/DI pattern.
|
||||
|
||||
### Known workaround
|
||||
Consumer avoids the config-reading `AcLoggerBase(string)` ctor on these platforms. DI-based `AddAcLoggerFactory<TLogger>` + `services.Configure<AcLoggerOptions>(...)` is the replacement (see LOGGING.md).
|
||||
|
||||
### Related TODO
|
||||
`LOGGING_TODO.md#todo-02`
|
||||
|
||||
## ISSUE-03: Two parallel logger-setup patterns
|
||||
|
||||
**Severity:** Minor (confusion, not functional) · **Status:** Documented · **Area:** LOGGING.md / consumer code
|
||||
|
||||
### Description
|
||||
Two ways to construct a logger coexist:
|
||||
1. **Config-reading:** `new Logger(categoryName)` → `AcLoggerBase` reads `AyCode:Logger` section via `AcEnv.AppConfiguration`, instantiates writers via `Activator.CreateInstance`
|
||||
2. **DI factory:** `services.Configure<AcLoggerOptions>(...)` + `services.AddAcLoggerFactory<TLogger>()` → `Func<string, TLogger>` resolved from DI, writers pulled from DI
|
||||
|
||||
Consumer picks per scenario; no automatic bridge. Risk: mixing patterns causes subtle failures (e.g. `MissingMethodException` — see ISSUE-01).
|
||||
|
||||
### Related TODO
|
||||
`LOGGING_TODO.md#todo-03`
|
||||
|
||||
## ISSUE-04: Default LogLevel diverges across the two setup paths
|
||||
|
||||
**Severity:** Minor (surprise, not broken) · **Status:** Open · **Area:** `AcLoggerBase` field initializer vs `AcLoggerOptions`
|
||||
|
||||
### Description
|
||||
The two logger-setup paths (see ISSUE-03) ship with **different default LogLevels**:
|
||||
- `AcLoggerBase.cs:18` — field initializer: `LogLevel = Error`
|
||||
- `AcLoggerOptions.cs:27` — DI options default: `LogLevel = Info`
|
||||
|
||||
Consequences:
|
||||
- Config-reading ctor WITHOUT an `AyCode:Logger:LogLevel` entry → silent `Error` minimum (loses Info/Warning in prod)
|
||||
- DI factory WITHOUT `services.Configure<AcLoggerOptions>(...)` → `Info` minimum
|
||||
|
||||
Same consumer code, same "no configuration" state, two different log volumes depending on which path was chosen.
|
||||
|
||||
### Root cause
|
||||
Historical: field initializer predates the Options class; Options was added as part of the DI-factory refactor with a developer-friendly `Info` default.
|
||||
|
||||
### Related TODO
|
||||
`LOGGING_TODO.md#todo-05`
|
||||
|
||||
## ISSUE-05: `AcConsoleLogWriter.Initialize()` runs twice on parameterless ctor
|
||||
|
||||
**Severity:** Minor (wasted work, not broken) · **Status:** Open · **Area:** `AcConsoleLogWriter` ctor chain
|
||||
|
||||
### Description
|
||||
`AcConsoleLogWriter.cs:14-17`:
|
||||
```csharp
|
||||
protected AcConsoleLogWriter() : this(null) // chains to (string?) ctor
|
||||
{
|
||||
Initialize(); // ← second call
|
||||
}
|
||||
|
||||
protected AcConsoleLogWriter(string? categoryName = null) : base(categoryName)
|
||||
{
|
||||
Initialize(); // ← first call (via chain)
|
||||
}
|
||||
```
|
||||
|
||||
Chain `: this(null)` invokes the `(string?)` ctor, which calls `Initialize()`. Control returns to the parameterless ctor body and calls `Initialize()` again. Net effect: `Console.ForegroundColor = ConsoleColor.White` set twice — harmless but reveals a copy-paste oversight.
|
||||
|
||||
### Fix direction
|
||||
Remove the second `Initialize()` call in the parameterless ctor body (the chain already covered it).
|
||||
|
||||
### Related TODO
|
||||
`LOGGING_TODO.md#todo-06`
|
||||
|
||||
## ISSUE-06: `ILogger.IsEnabled(MsLogLevel.None)` incorrectly reports enabled
|
||||
|
||||
**Severity:** Low (semantic bug, rare path) · **Status:** Open · **Area:** `AcLoggerBase.IsEnabled` + `MapFromMsLogLevel`
|
||||
|
||||
### Description
|
||||
`AcLoggerBase.cs:184-188`:
|
||||
```csharp
|
||||
public bool IsEnabled(MsLogLevel logLevel)
|
||||
{
|
||||
var acLogLevel = MapFromMsLogLevel(logLevel);
|
||||
return LogLevel <= acLogLevel;
|
||||
}
|
||||
```
|
||||
|
||||
`MapFromMsLogLevel(MsLogLevel.None) → AcLogLevel.Disabled (byte 255)`. For any instance `LogLevel` (max `Error=25`), `LogLevel <= 255` is **always true** → `IsEnabled(None)` returns `true` despite `None` meaning "off".
|
||||
|
||||
The subsequent `Log<TState>` switch has `case MsLogLevel.None: default: break;` so no actual write occurs, BUT:
|
||||
- The MS-logging framework may query `IsEnabled` as a gate before calling `Log` — hosts using that pattern get misleading "enabled" signals
|
||||
- Any caller that inspects `IsEnabled(None)` (diagnostics, test assertions) gets the wrong answer
|
||||
|
||||
### Fix direction
|
||||
Either:
|
||||
- **(a)** Explicit `if (logLevel == MsLogLevel.None) return false;` at the top of `IsEnabled`
|
||||
- **(b)** Change the comparison to strict `<` with `Disabled` as sentinel (larger refactor — affects semantic of `Disabled` elsewhere)
|
||||
|
||||
### Related TODO
|
||||
`LOGGING_TODO.md#todo-07`
|
||||
|
||||
## ISSUE-07: Misleading inline comment in `AcLoggerBase.Log<TState>`
|
||||
|
||||
**Severity:** Trivial (doc-only) · **Status:** Open · **Area:** `AcLoggerBase.cs:210-211`
|
||||
|
||||
### Description
|
||||
```csharp
|
||||
// Use eventId.Name as memberName if available, otherwise null (will show as empty, not "Log")
|
||||
var memberName = !string.IsNullOrEmpty(eventId.Name) ? $"{eventId.Name}:{eventId.Id}" : "Log";
|
||||
```
|
||||
|
||||
Comment says fallback is `null` (empty display), the code assigns `"Log"`. Contradicts both the doc in `LOGGING.md#CallerMemberName Auto-Capture` and the actual behaviour observed in logs.
|
||||
|
||||
### Fix direction
|
||||
Update comment to match code: `// Use eventId.Name:eventId.Id if Name is set, otherwise fallback to "Log" per LOGGING.md convention`.
|
||||
|
||||
### Related TODO
|
||||
Folded into `LOGGING_TODO.md#todo-08` (cleanup batch).
|
||||
|
||||
## Evaluated review findings — NOT bugs (by-design)
|
||||
|
||||
Cross-session review (2026-04) flagged several items that are **intentional defense-in-depth / YAGNI** and should NOT be "fixed":
|
||||
|
||||
- **"Triple LogLevel gate on the ILogger.Log path is redundant"** — NOT redundant. `Info()`/`Debug()` can be called directly (not only via `ILogger.Log`), so each named method MUST own its gate. Per-writer gate is the documented Gate-2 (see LOGGING.md "Two independent level gates"). Removing any gate breaks a real path.
|
||||
- **"`AcTextLogWriterBase.GetDiagnosticText` defensive level-check is redundant"** — same rationale; direct callers exist.
|
||||
- **"`IAcLoggerBase` narrow interface (no dynamic LogLevel setter via interface)"** — YAGNI. Consumers needing dynamic level can cast to `AcLoggerBase`. Widening the interface for a rare scenario pollutes the abstraction.
|
||||
- **"`protected List<IAcLogWriterBase> LogWriters` is mutable"** — acceptable; `GetWriters` returns a defensive copy (`[.. LogWriters]`), direct mutation is only reachable from derived classes (which control their own ctor-time population).
|
||||
|
|
@ -0,0 +1,85 @@
|
|||
# Logger — TODO
|
||||
|
||||
## Priority legend
|
||||
- **P0** blocker · **P1** important · **P2** nice-to-have · **P3** idea
|
||||
|
||||
---
|
||||
|
||||
## TODO-01: Fix the writer-ctor mismatch for DI-injected writers
|
||||
**Priority:** P2 · **Type:** Bug fix · **Related:** `LOGGING_ISSUES.md#issue-01`
|
||||
|
||||
Options:
|
||||
- **A)** Provide a fallback `(AppType, LogLevel, string?)` ctor on consumer writers that have DI-heavy primary ctors, with DI resolution via service-locator
|
||||
- **B)** Skip writers that don't match the `Activator`-ctor signature, with a single consolidated warning (not per-init spam)
|
||||
- **C)** Deprecate the config-reading writer-instantiation path entirely in favour of DI registration
|
||||
|
||||
Eliminate the per-startup Console.Error noise regardless.
|
||||
|
||||
## TODO-02: Expose `AcEnv.AppConfiguration` setter for consumer init
|
||||
**Priority:** P2 · **Type:** Feature · **Related:** `LOGGING_ISSUES.md#issue-02`
|
||||
|
||||
Allow `AcEnv.SetConfiguration(IConfiguration)` so consumer `Program.cs` can do `AcEnv.SetConfiguration(builder.Configuration)` at startup. Enables config-reading pattern on MAUI/WASM without filesystem assumptions. Backward-compat: fall back to filesystem if no explicit set.
|
||||
|
||||
## TODO-03: Unify or clearly separate config-reading and DI-based patterns
|
||||
**Priority:** P2 · **Type:** Docs / Refactor · **Related:** `LOGGING_ISSUES.md#issue-03`
|
||||
|
||||
Decide the canonical direction:
|
||||
- **(a)** Deprecate config-reading pattern → all consumers migrate to DI factory
|
||||
- **(b)** Keep both, with compile-time guidance (analyzer / XML doc `[Obsolete]` hints / decision tree in LOGGING.md)
|
||||
- **(c)** Merge: DI-factory internally falls back to config-reading when `TLogger` doesn't match the `Activator` ctor shape
|
||||
|
||||
## TODO-04: Per-writer LogLevel via appsettings
|
||||
**Priority:** P2 · **Type:** Feature
|
||||
|
||||
Extend `AcLoggerOptions` with per-writer LogLevel overrides. Example shape:
|
||||
```json
|
||||
"AyCode": {
|
||||
"Logger": {
|
||||
"LogLevel": "Info",
|
||||
"WriterLevels": { "BrowserConsoleLogWriter": "Debug", "SignaRClientLogItemWriter": "Warning" }
|
||||
}
|
||||
}
|
||||
```
|
||||
Factory applies overrides when constructing writers. Currently writer-LogLevel is hardcoded in writer ctors.
|
||||
|
||||
## TODO-05: Unify default LogLevel across setup paths
|
||||
**Priority:** P1 · **Type:** Behaviour decision + cleanup · **Related:** `LOGGING_ISSUES.md#issue-04`
|
||||
|
||||
Decide which default wins (`AcLoggerBase.cs:18` = `Error` vs `AcLoggerOptions.cs:27` = `Info`) and make both paths consistent. Recommendation: **`Info`** — matches the DI/Options path already in use for modern consumers (MAUI, WASM, ASP.NET Core) and is the common developer expectation.
|
||||
|
||||
Change: drop the field initializer in `AcLoggerBase` (or set it to `Info`). Update LOGGING.md with a single "Default LogLevel = Info" line so this can't drift again.
|
||||
|
||||
## TODO-06: Fix `AcConsoleLogWriter.Initialize()` double-run
|
||||
**Priority:** P3 · **Type:** Bug fix · **Related:** `LOGGING_ISSUES.md#issue-05`
|
||||
|
||||
Remove the redundant `Initialize()` call from the parameterless ctor body — the `: this(null)` chain already invokes it. No behaviour change; removes wasted `Console.ForegroundColor` assignment.
|
||||
|
||||
## TODO-07: Fix `ILogger.IsEnabled(MsLogLevel.None)` wrongly reporting enabled
|
||||
**Priority:** P2 · **Type:** Bug fix · **Related:** `LOGGING_ISSUES.md#issue-06`
|
||||
|
||||
Add `if (logLevel == MsLogLevel.None) return false;` at the top of `AcLoggerBase.IsEnabled`. One-line fix. Optionally add a unit test covering all six MS LogLevel values → expected bool.
|
||||
|
||||
## TODO-08: Cleanup batch (low-risk micro-refactor, one commit)
|
||||
**Priority:** P3 · **Type:** Cleanup · **Related:** `LOGGING_ISSUES.md#issue-07`
|
||||
|
||||
Single commit covering:
|
||||
- Remove unused usings in `AcLoggerBase.cs` (`System.Security.AccessControl`, `System.Net.Mime.MediaTypeNames`)
|
||||
- Extract the default-category magic string `"..."` (at least 3 occurrences — `AcLoggerBase.cs:36,76`, writer side) into a public const `AcLoggerBase.DefaultCategory = "..."` with XML doc explaining it's the "unknown category" sentinel, NOT a magic string. Update all call-sites.
|
||||
- Fix misleading comment at `AcLoggerBase.cs:210` — current comment says "fallback null" but code assigns `"Log"`. Make comment match code.
|
||||
- Decide on the commented-out batch block in `AcLogItemWriterBase.cs:90-119`: either delete (git history preserves) or convert to a single `// TODO: see LOGGING_TODO.md#todo-XX` marker with a new TODO entry for the batch work.
|
||||
|
||||
## TODO-09: Fail-fast ctor validation in `AddAcLoggerFactory<TLogger>`
|
||||
**Priority:** P2 · **Type:** Feature
|
||||
|
||||
`AcLoggerServiceExtensions.AddAcLoggerFactory<TLogger>` uses `Activator.CreateInstance(typeof(TLogger), AppType, LogLevel, categoryName, writers)` only on first logger resolution. Ctor-mismatch therefore surfaces only at first log call, not at `services.BuildServiceProvider()` time.
|
||||
|
||||
Add a one-time reflection check at registration: `typeof(TLogger).GetConstructor([typeof(AppType), typeof(LogLevel), typeof(string), typeof(IAcLogWriterBase[])])` — if null, throw `InvalidOperationException` with the expected signature in the message. Cost: one reflection call per registration. Benefit: app fails to start (loud) instead of logging being silently broken (quiet).
|
||||
|
||||
## TODO-10: Writer exception isolation
|
||||
**Priority:** P2 · **Type:** Resilience
|
||||
|
||||
Currently `AcLoggerBase.Info(...)` and friends do `LogWriters.ForEach(x => x.Info(...))`. A throwing writer (e.g. `SignaRClientLogItemWriter` during network blip, `AcDbLogItemWriter` during a DB outage) takes down the fan-out → subsequent writers in the list never see the message.
|
||||
|
||||
Wrap each writer invocation in `try { ... } catch (Exception ex) { Console.Error.WriteLine($"Writer {x.GetType().Name} failed: {ex.Message}"); }`. Optional: escalate to a circuit-breaker pattern (N failures → disable that writer for M seconds) to avoid Console.Error flood on persistent outages.
|
||||
|
||||
Must NOT log to `AcLoggerBase` itself inside the catch (reentrancy / infinite loop risk).
|
||||
|
|
@ -152,9 +152,7 @@ GetParameterValues(ParameterInfo[]):
|
|||
|
||||
Type-guided deserialization — each parameter is individually serialized/deserialized with its concrete type, avoiding the `object[]` → dictionary problem of untyped binary deserialization.
|
||||
|
||||
**Perf concern:** Per-parameter `ToBinary()`/`BinaryTo(Type)` = N× context pool acquire/release + N× type-dispatch (ThreadLocal + ConcurrentDictionary cache). For many small primitives (int, bool, string) the per-call overhead may exceed a single bulk serialization call. Complex objects benefit clearly. If benchmarks show regression vs old JSON path, a batch fast-path (single serialization context for all params) should be added.
|
||||
|
||||
**Limitation:** Parameter serialization/deserialization is currently AcBinary only (`ToBinary()`/`BinaryTo()`). JSON support would require dispatching on serializer type in `SignalParams` methods + AcJsonSerializer reference.
|
||||
> Known concerns and limitations on parameter serialization (per-parameter overhead, AcBinary-only) are tracked in `SIGNALR_ISSUES.md` under `PROTO-2` and `PROTO-3`.
|
||||
|
||||
## Response Patterns
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,34 @@
|
|||
# Binary Hub Protocol (wire) — Known Issues
|
||||
|
||||
For planned/actionable work see `SIGNALR_BINARY_PROTOCOL_TODO.md`.
|
||||
For higher-level SignalR abstractions see `SIGNALR_ISSUES.md`.
|
||||
|
||||
## ISSUE-01: AsyncSegment send-path unsupported on WebAssembly
|
||||
|
||||
**Severity:** Major (on WASM) · **Status:** Workaround-in-place · **Area:** `AsyncPipeWriterOutput` / WASM runtime
|
||||
|
||||
### Description
|
||||
`AsyncPipeWriterOutput.SyncAwaitFlush` uses `Task.Wait(timeout)` — deadlocks the single-threaded WASM UI thread. Therefore, WASM clients cannot SEND with `AsyncSegment` mode.
|
||||
|
||||
### Known workaround (multi-layer)
|
||||
1. **`AcBinaryHubProtocolOptions.Validate()`** throws `PlatformNotSupportedException` if WASM + AsyncSegment combination is requested → prevents deadlock
|
||||
2. **Consumer code-level safety-net** downgrades `AsyncSegment → Segment` on WASM (see `FruitBankHybrid.Web.Client/Program.cs` Configure lambda)
|
||||
3. **Receive-path** on WASM is fully supported — `SegmentBufferReaderInput` with synchronous fallback at `CHUNK_END` means WASM clients CAN receive AsyncSegment-chunked data from a non-WASM sender, they just cannot send AsyncSegment themselves
|
||||
|
||||
### Related TODO
|
||||
None — architectural constraint of browser WASM threading model.
|
||||
|
||||
## ISSUE-02: StaticWebAssets SDK "Illegal characters" noise (consumer build)
|
||||
|
||||
**Severity:** Cosmetic (non-blocking) · **Status:** Upstream SDK limitation · **Area:** SDK, not our code
|
||||
|
||||
### Description
|
||||
Consumer projects using `Microsoft.NET.Sdk.BlazorWebAssembly` may see "Illegal characters in path" errors in the VS design-time error list. Originates from the SDK's `DefineStaticWebAssets` task calling legacy `FileIOPermission.EmulateFileIOPermissionChecks`, which is stricter than NTFS (e.g., double spaces or certain path patterns in any wwwroot asset trigger it).
|
||||
|
||||
Build succeeds, app runs — the "error" is error-list-only.
|
||||
|
||||
### Known workaround
|
||||
Ignore the error-list entry. If it becomes truly blocking, inspect wwwroot (including transitive from RCL projects) for problematic filenames (double spaces, control chars, reserved chars).
|
||||
|
||||
### Related TODO
|
||||
None — upstream SDK fix required.
|
||||
|
|
@ -0,0 +1,46 @@
|
|||
# Binary Hub Protocol (wire) — TODO
|
||||
|
||||
## Priority legend
|
||||
- **P0** blocker · **P1** important · **P2** nice-to-have · **P3** idea
|
||||
|
||||
---
|
||||
|
||||
## TODO-01: SegmentBufferReader isolated unit tests
|
||||
**Priority:** P1 · **Type:** Test coverage
|
||||
|
||||
Original `vast-brewing-moonbeam` refactor plan (chunked receive-path) listed these tests in its verification section; they were never written. Needed:
|
||||
- `Write` / `TryAdvanceSegment` / `Complete` basic contract
|
||||
- Producer-consumer concurrency (grow-buffer handoff race)
|
||||
- Missed-signal double-check pattern under `ManualResetEventSlim` reset
|
||||
- `Dispose` lifecycle (buffer pool return, old-buffer cleanup)
|
||||
|
||||
## TODO-02: Chunked protocol integration test
|
||||
**Priority:** P1 · **Type:** Test coverage
|
||||
|
||||
End-to-end round-trip:
|
||||
- `WriteMessageChunked(obj, AsyncSegment)` → bytes
|
||||
- `TryParseMessage(bytes)` → reconstructed object
|
||||
- Assert structural equality with original
|
||||
|
||||
Cover asymmetric cases:
|
||||
- AsyncSegment sender, Segment receiver
|
||||
- Bytes sender, AsyncSegment receiver
|
||||
- WASM-downgraded sender (Segment), server AsyncSegment receiver
|
||||
|
||||
## TODO-03: `BinaryProtocolMode.Auto` wire-detection implementation
|
||||
**Priority:** P3 · **Type:** Feature · **Related:** `SIGNALR_TODO.md#todo-04`
|
||||
|
||||
Client-side adaptive mode: on first received message, inspect first payload byte:
|
||||
- `CHUNK_START (200)` → peer uses AsyncSegment → match it on subsequent sends (subject to local-platform constraint — WASM safety-net overrides to Segment)
|
||||
- Non-chunked → stay with Segment/Bytes
|
||||
|
||||
Per-`HubConnection` state. Requires changes to `BinaryProtocolMode` enum + detection wiring in `TryParseMessage`.
|
||||
|
||||
## TODO-04: SignalR handshake-extension for upfront mode negotiation
|
||||
**Priority:** P3 · **Type:** Feature · **Related:** TODO-03
|
||||
|
||||
Alternative to wire-detection: use SignalR handshake message's `extensions` JSON field to carry protocol-capability info — e.g.
|
||||
```json
|
||||
{ "protocol": "acbinary", "version": 1, "extensions": { "acbinary": { "preferredMode": "AsyncSegment" } } }
|
||||
```
|
||||
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.
|
||||
|
|
@ -53,3 +53,44 @@ Transport max message size (30MB) and application buffer (30MB) are hardcoded. S
|
|||
The `GetAll` path uses `IsRawBytesData = true` to receive raw `byte[]` from the protocol, then deserializes into the existing list via `PopulateMerge`. This avoids allocating a temporary `List<T>` for merge. The extra copy (pipe → byte[]) is the trade-off.
|
||||
|
||||
**Possible optimization:** Direct typed deserialization with merge support in the deserializer (PopulateMerge from `ReadOnlySequence<byte>`). Requires deserializer API changes.
|
||||
|
||||
## Client-side Setup & DI
|
||||
|
||||
### CONN-1: HubConnectionBuilder inner DI isolation
|
||||
|
||||
**Status:** Workaround-in-place (dedicated options-passing overload)
|
||||
**Affects:** Consumer client setup in `Program.cs` (MAUI, WASM, ASP.NET Core server prerender)
|
||||
|
||||
`HubConnectionBuilder.Services` is a separate `IServiceCollection` from the outer host DI. `services.Configure<AcBinaryHubProtocolOptions>(...)` registered in the outer container does NOT flow into `HubConnectionBuilder.Services`. Calling `hubBuilder.AddAcBinaryProtocol()` with no args silently falls back to default options.
|
||||
|
||||
**Known workaround:** Use the overload `AddAcBinaryProtocol(IHubConnectionBuilder, AcBinaryHubProtocolOptions, Action<...>? = null)` that accepts pre-resolved options explicitly. Canonical consumer pattern:
|
||||
```csharp
|
||||
var protocolOpts = sp.GetRequiredService<IOptions<AcBinaryHubProtocolOptions>>().Value;
|
||||
hubBuilder.AddAcBinaryProtocol(protocolOpts);
|
||||
```
|
||||
|
||||
## Dispatch
|
||||
|
||||
### DISPATCH-1: First-call null response (observed)
|
||||
|
||||
**Status:** Open — not diagnosed
|
||||
**Affects:** `PostDataAsync<T>` awaiter / OnReceiveMessage → pending-request correlation
|
||||
|
||||
Observed symptom: first `GetProductDtos_80`-style call returns null despite server serializing and sending a valid ~80KB chunked response. Second call (client-side retry) works normally.
|
||||
|
||||
Log timeline:
|
||||
- Server: `Serialize end (chunked) dataBytes=80571 chunkCount=20`
|
||||
- Client: `Deserialize end (chunked)` — successful
|
||||
- Client: `OnReceiveMessage ... requestId=5`
|
||||
- Client: ~410ms later — `Client received null response. ... requestId=5`
|
||||
- Client: auto-retry (requestId=6) → full 338 items
|
||||
|
||||
Hypothesis (unverified): `PostDataAsync<T>` awaiter's null-mapping path misroutes the parsed result, or `requestId → Task<T>` correlation has a race on the first response of a fresh connection. Client auto-retry hides the user-visible impact.
|
||||
|
||||
**Related TODO:** `SIGNALR_TODO.md#todo-01`
|
||||
|
||||
## Cross-cutting (also tracked in serializer-side docs)
|
||||
|
||||
### XCUT-1: JSON-in-Binary request parameters — cross-ref
|
||||
|
||||
Same tech debt as `../../AyCode.Core/docs/BINARY_ISSUES.md#xcut-1`. Planned replacement: migrate client→server request parameters from JSON-in-Binary envelope to direct Binary serialization. Coordinated change across all consuming projects.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,31 @@
|
|||
# SignalR — TODO
|
||||
|
||||
## Priority legend
|
||||
- **P0** blocker · **P1** important · **P2** nice-to-have · **P3** idea
|
||||
|
||||
---
|
||||
|
||||
## TODO-01: Diagnose first-call null in PostDataAsync<T>
|
||||
**Priority:** P2 · **Type:** Investigation · **Related:** `SIGNALR_ISSUES.md#issue-02`
|
||||
|
||||
Reproduce the `GetProductDtos_80`-style first-call null. Add trace logs to `PostDataAsync<T>` awaiter path and `OnReceiveMessage → pending request` dictionary lookup. Verify `requestId → Task<T>` correlation on the very first chunked response of a fresh connection.
|
||||
|
||||
## TODO-02: Document asymmetric send/receive capability
|
||||
**Priority:** P1 · **Type:** Docs
|
||||
|
||||
Current behaviour: sender selects `BinaryProtocolMode` independently; receiver detects the wire format from the first byte (`CHUNK_START=200` → chunked path; else non-chunked). This means client and server can run DIFFERENT `ProtocolMode` settings independently — a core feature amplifying interoperability.
|
||||
|
||||
Document in `SIGNALR_BINARY_PROTOCOL.md` as a dedicated "Asymmetric send/receive contract" section. Key selling points:
|
||||
- WASM client + AsyncSegment server → works (WASM downgrades send, receives chunked happily)
|
||||
- Third-party client on NuGet can pick any mode → server doesn't care
|
||||
- Gradual mode rollouts possible (no synchronized deploy)
|
||||
|
||||
## TODO-03: Code-level guard for FlushTimeout < ClientTimeoutInterval
|
||||
**Priority:** P2 · **Type:** Feature
|
||||
|
||||
`AcBinaryHubProtocolOptions.Validate()` currently documents (in XML doc) that `FlushTimeout` should be less than the SignalR `HubOptions.ClientTimeoutInterval`, but there is no code-level check. Add validation at protocol registration time — if both options are resolvable from the DI scope, verify the constraint and emit a startup warning (or throw, pending decision).
|
||||
|
||||
## TODO-04: `BinaryProtocolMode.Auto` — adaptive send-mode
|
||||
**Priority:** P3 · **Type:** Feature · **Related:** `SIGNALR_BINARY_PROTOCOL_TODO.md#todo-03`
|
||||
|
||||
Design: on first received message, inspect first byte to determine peer's send format. On subsequent sends, match it (subject to local-platform constraints, e.g. WASM never actually sends AsyncSegment). Per-`HubConnection` state. Optional upfront handshake-extension negotiation as an alternative — see wire-level TODO.
|
||||
|
|
@ -33,12 +33,7 @@ See `AyCode.Services/docs/SIGNALR.md` for full architecture documentation.
|
|||
|
||||
### ⚠️ Temporary: JSON-in-Binary Request Parameters
|
||||
|
||||
Client→server request parameters currently use a JSON→Binary→JSON round-trip:
|
||||
1. `SignalPostJsonDataMessage<T>` serializes `PostData` to JSON string (`PostDataJson`)
|
||||
2. The JSON-containing message is wrapped in a Binary envelope
|
||||
3. Server deserializes Binary → extracts JSON string → parses JSON per parameter
|
||||
|
||||
This is **planned for replacement** with direct Binary parameter serialization (matching how responses already work). Do not attempt to fix this as a side-effect of other work — it requires coordinated client+server+consuming-project changes.
|
||||
Client→server request parameters currently use a JSON-inside-Binary envelope — a cross-cutting tech debt planned for migration to pure Binary. Details in `AyCode.Core/docs/BINARY_ISSUES.md#xcut-1` + `AyCode.Services/docs/SIGNALR_ISSUES.md#xcut-1`. Migration is tracked in `BINARY_TODO.md#todo-01`. Do NOT attempt as a side-effect of unrelated work — requires coordinated client+server+consuming-project changes.
|
||||
|
||||
## Testing
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue