f0455a1e45
Generated design docs and implementation plans via Codex for: - Batch 6: Opts package-level functions - Batch 7: Opts class methods + Reload - Batch 9: Auth, DirStore, OCSP foundations - Batch 10: OCSP Cache + JS Events - Batch 11: FileStore Init - Batch 12: FileStore Recovery - Batch 16: Client Core (first half) - Batch 17: Client Core (second half) All plans include mandatory verification protocol and anti-stub guardrails. Updated batches.md with file paths and planned status.
127 lines
5.9 KiB
Markdown
127 lines
5.9 KiB
Markdown
# Batch 16 (Client Core first half) Design
|
|
|
|
**Date:** 2026-02-27
|
|
**Scope:** Design only for Batch 16 (`server/client.go` first-half mappings): 30 features and 1 tracked test.
|
|
|
|
## Context Snapshot
|
|
|
|
Batch metadata from PortTracker:
|
|
|
|
- Batch ID: `16`
|
|
- Name: `Client Core (first half)`
|
|
- Dependencies: batches `2`, `3`, `4`
|
|
- Go file: `server/client.go`
|
|
- Features: `30` (all currently `deferred`)
|
|
- Tests: `1` (`#2859`, currently `deferred`)
|
|
|
|
Tracker status snapshot (`report summary`):
|
|
|
|
- Features: `1271 verified`, `2377 deferred`, `24 n_a`, `1 stub`
|
|
- Unit tests: `430 verified`, `2640 deferred`, `187 n_a`
|
|
- Overall progress: `1924 / 6942 (27.7%)`
|
|
|
|
## Problem Statement
|
|
|
|
Batch 16 mixes:
|
|
|
|
1. Small deterministic helpers (flag operations, type helpers, permission shaping).
|
|
2. Mid-complexity outbound pipeline methods (buffer collapse, flush, timeout handling, close-path state).
|
|
3. Parser-adjacent methods (`processPub`, `processHeaderPub`, `splitArg`, `parseSub`, `processSub*`) that touch broader server/account state.
|
|
|
|
The key risk is fake progress via placeholder ports. The design must enforce behavior-faithful implementation with explicit defer decisions where later infrastructure is truly required.
|
|
|
|
## Working Set Breakdown
|
|
|
|
### Group A (19 features, deterministic/helper-focused, <= line 1300)
|
|
|
|
`387, 388, 389, 390, 391, 392, 393, 394, 395, 396, 397, 398, 402, 411, 420, 421, 422, 423, 424`
|
|
|
|
### Group B (11 features, outbound + parser-facing, lines 1608-3046)
|
|
|
|
`429, 430, 431, 432, 456, 471, 472, 473, 474, 475, 476`
|
|
|
|
### Batch Test
|
|
|
|
- `2859` `TestRouteSlowConsumerRecover` -> mapped .NET target:
|
|
`RouteHandlerTests.RouteSlowConsumerRecover_ShouldSucceed`
|
|
|
|
## Current Code Findings
|
|
|
|
1. `ClientConnection.cs` already contains partial equivalents for some methods, but several mapped method names do not exist yet (`PublicPermissions`, `MergeDenyPermissions`, `FlushOutbound`, `HandleWriteTimeout`, `QueueOutbound`, `ProcessSubEx`, etc.).
|
|
2. `WriteTimeoutPolicy` is currently an enum; Go-style `String()` behavior exists via extension method (`ToVarzString`) rather than mapped name.
|
|
3. `clientFlag.*` / `readCacheFlag.*` features are mapped to `ClientFlags` and `ReadCacheFlags`, but those are enums in C#, which cannot host instance methods.
|
|
4. `ProtocolParser` already has high-quality static `ProcessPub` and `ProcessHeaderPub` parsing logic; Batch 16 should reuse this rather than duplicate parser internals.
|
|
5. Tracked test `#2859` depends on multiple non-Batch-16 features; at least one dependency (`#1207 fileStore.stop`) is still deferred, so test completion may remain blocked even after Batch 16 feature work.
|
|
|
|
## Approaches
|
|
|
|
### Approach A: Full in-place parity in one sweep
|
|
|
|
Port all 30 features directly in `ClientConnection.cs` before any status updates.
|
|
|
|
- Pros: Single execution wave, fewer intermediate commits.
|
|
- Cons: High regression risk, weak isolation, high chance of hidden stubs.
|
|
|
|
### Approach B: Thin wrappers + audit overrides
|
|
|
|
Add minimal wrappers, use status overrides for non-matching mappings, defer heavy logic.
|
|
|
|
- Pros: Fastest movement.
|
|
- Cons: Creates weak verification evidence and makes future maintenance harder.
|
|
|
|
### Approach C (Recommended): Two-group staged parity with explicit mapping hygiene and hard verification gates
|
|
|
|
Implement deterministic helper features first (Group A), then outbound/parser features (Group B), with strict per-feature evidence and chunked status updates. Treat test `#2859` as execute-or-defer based on runtime feasibility.
|
|
|
|
- Pros: Lowers risk, produces auditable evidence, prevents stub creep.
|
|
- Cons: Slightly more upfront discipline and tracker hygiene.
|
|
|
|
## Recommended Design
|
|
|
|
### 1. Surface and Mapping Alignment
|
|
|
|
- Keep `ClientConnection` as primary implementation location for Batch 16 methods.
|
|
- Reuse existing helper types where already idiomatic (`NbPool`, `ProtocolParser`) through `ClientConnection` wrappers.
|
|
- For enum-based mapped methods (`ClientFlags`, `ReadCacheFlags`, `WriteTimeoutPolicy.String`), perform explicit mapping reconciliation to method-hosting helper types when needed, rather than distorting core domain types.
|
|
|
|
### 2. Behavior Porting Strategy
|
|
|
|
- Port from Go source line ranges per feature ID, preserving observable behavior, lock expectations, and error paths.
|
|
- Prefer deterministic, unit-testable slices:
|
|
- Flag transitions and deny-permission merge semantics.
|
|
- Outbound queue accounting and timeout-close behavior.
|
|
- Pub/sub argument parsing wrappers around existing parser primitives.
|
|
|
|
### 3. Verification-First Execution Model
|
|
|
|
- Per-feature red/green loop with focused tests and build checks.
|
|
- Group-level gates (stub scan, full build, related test classes) before status promotions.
|
|
- Max 15 IDs per status update command, with evidence for every chunk.
|
|
|
|
### 4. Tracked Test Strategy (`#2859`)
|
|
|
|
- Attempt realistic port only if feature dependencies and harness capabilities are sufficient.
|
|
- If infrastructure dependencies remain unmet, keep `deferred` with explicit blocker reason; no placeholder pass test is allowed.
|
|
|
|
## Error Handling and Deferral Rules
|
|
|
|
1. If a feature requires unavailable infrastructure (for example full route/file-store interactions), mark it `deferred` with a concrete reason.
|
|
2. Never introduce fake success patterns (`NotImplementedException` stubs, TODO-only placeholders, trivial always-pass assertions).
|
|
3. Do not promote `verified` unless build and related tests are green with captured evidence.
|
|
|
|
## Success Criteria
|
|
|
|
1. Batch 16 features are either:
|
|
- behavior-faithfully implemented and verified, or
|
|
- explicitly deferred with specific blocker notes.
|
|
2. No stub patterns remain in touched source/test files.
|
|
3. Status transitions are evidence-backed and chunked (`<=15` IDs/update).
|
|
4. Batch test `#2859` is either genuinely verified or explicitly deferred with documented dependency blockers.
|
|
|
|
## Non-Goals
|
|
|
|
1. No execution in this design document.
|
|
2. No unrelated refactors outside Batch 16 scope.
|
|
3. No forced integration test completion through fragile or fake harnesses.
|
|
|