The AdminUI "Test Connect" probe for the Sql driver: open a connection, run
the dialect's LivenessSql (SELECT 1) under a linked-CTS deadline, return
green with latency or red with a reason. Implements IDriverProbe; never
throws (malformed JSON, missing connectionStringRef, unprovisioned
connection string, a provider open failure, timeout, cancellation all
become red results).
Parses the SAME factory DTO with the SAME JsonStringEnumConverter as
SqlDriverFactoryExtensions (R2-11 factory parity, mirroring
ModbusDriverProbe) and resolves connectionStringRef the same way, so a
config that Test-Connects is the config that Deploys.
Credential hygiene: the resolved connection string never reaches the result
message. A provider exception can embed the data source in its OWN message
(the real SqlException shape), so the catch-all names the exception TYPE
only, never ex.Message. The regression test's fake connection embeds the
connection string in its thrown message specifically so surfacing ex.Message
would leak it — verified load-bearing by breaking the guard and watching it
go red.
ForTest injects factory+dialect for the offline SQLite fixture path.
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
SqlDriverFactoryExtensions turns a deployed DriverConfig blob into a live
SqlDriver, and SqlConnectionStringResolver resolves the authored
connectionStringRef NAME from Sql__ConnectionStrings__<ref> so credentials
never ride in a config blob (design §8.2).
Three behaviours worth naming:
- String-enum guard. JsonOptions carries JsonStringEnumConverter +
camelCase, so `provider` parses whether authored as a name or an ordinal
and always round-trips as the NAME — the systemic AdminUI defect where a
page serialised an enum numerically against a string-typed DTO.
- Config validation lands here because nothing below does it.
operationTimeout must be STRICTLY greater than commandTimeout (design
§8.3); the reader and the driver stay deliberately usable with the pair
inverted so the frozen-database tests can prove the client-side bound
fires. Non-positive timeouts / poll interval and maxConcurrentGroups < 1
are rejected too.
- Credential hygiene. The resolved connection string reaches the provider
and nothing else: messages name the ref, the environment variable, or
SqlDriver.Endpoint's credential-free server/database rendering. Both a
log-leak and an exception-leak test cover it.
DTO changes:
- Adds RawTags (List<RawTagEntry>), following the Modbus precedent — the
deploy artifact delivers a driver's tags this way and the DTO had no way
to receive them, so an authored Sql tag could never reach the driver.
- Deletes SqlProbeDto + the `probe` key. It had no consumer: SqlDriver was
specified without a background probe loop, and deliberately so — its
IHostConnectivityProbe state is a by-product of the Initialize liveness
check and of every poll, i.e. a statement about traffic that actually
happened rather than a synthetic ping. On-demand connectivity is Task
10's SqlDriverProbe. Shipping a config key that silently does nothing is
worse than not shipping it; UnmappedMemberHandling.Skip means a blob that
still carries `probe` keeps parsing.
allowWrites stays inert (SqlDriver implements no write capability) but an
authored `true` now WARNS rather than passing silently — the flag cannot do
harm, an operator who believes writes are enabled can.
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
Three review findings against SqlPollReader (967e5140).
1. WideRow's where-pair form emits no ORDER BY and no row limit, so a
selector that is not actually unique made the reader publish an
arbitrary, storage-order-dependent row — silently, unlike IndexByKey's
duplicate-key warning. Row selection moves out of MapMember into a new
SelectWideRow, which is called once per group and emits the same
rate-limited contract warning (naming table, selector and row count).
The row-limit half of the suggested fix was deliberately NOT taken: a
TOP 1 with no ORDER BY is not deterministic either, and it would
destroy the Rows.Count > 1 evidence the warning depends on, turning a
loud misconfiguration into a permanently silent one. Rationale is in
SelectWideRow's docs. No SQL changed, so no planner golden string moved.
2. The "a frozen database can never accumulate more than
maxConcurrentGroups connections" claim was asserted in the docs and
untested. ReadAsync_aTimedOutGroupKeepsItsConcurrencySlotUntilTheQuery-
TrulyFinishes wedges a group behind an EXCLUSIVE transaction, times it
out, and proves the slot is still held (a second poll on a cap of 1
times out on the gate without ever reaching the factory) and is handed
back only once the abandoned query truly finishes. Moving the release
from the work to the waiter fails this test and no other.
3. Task.Run thread-pool starvation: documented, not changed.
Microsoft.Data.SqlClient's async path parks no pool thread on a frozen
server, so the driver cannot starve itself; LongRunning would cost a
dedicated OS thread per group per poll forever to insure against a
provider this driver does not ship. RunGroupAsync now records the
decision and what a BadTimeout does and does not mean, for the
blackhole gate.
Also: the timeout warning now names the group's table, so a BadTimeout
storm identifies which source is frozen.
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
SqlDriver implements IDriver / ITagDiscovery / IReadable / ISubscribable /
IHostConnectivityProbe / IAsyncDisposable over the landed SqlPollReader,
mirroring ModbusDriver. IWritable is deliberately absent: v1 is read-only
structurally, and every discovered variable is SecurityClass=ViewOnly.
The shell classifies poll outcomes because nothing below it does. The reader
honours IReadable literally — it throws only when the database is unreachable
and Bad-codes everything else — so a frozen database returns all-BadTimeout
snapshots through a perfectly successful PollGroupEngine tick. Without
ObservePollOutcome the driver would report Healthy while every value was Bad.
Connection-class codes (BadTimeout / BadCommunicationError) degrade health and
report the host Stopped; authoring-class codes (unresolvable RawPath, absent
row, type mismatch) change nothing, so a tag typo never reports the database
down. It deliberately does NOT synthesise an exception to earn engine backoff:
the engine's exception path publishes nothing, which would cost clients the
Bad quality the reader went out of its way to produce.
Initialize builds the authored RawPath table first (pure, cannot fail; a
malformed TagConfig is logged and skipped) then verifies liveness over one
open-use-dispose connection bounded by wall clock as well as by token (the
R2-01 lesson) — failure records Faulted and rethrows so DriverInstanceActor
retries. The connection string is never logged: a credential-free
server/database Endpoint is the only rendering that reaches a log, LastError,
or the host status.
DriverTypeNames.Sql is NOT added here — DriverTypeNamesGuardTests asserts
bidirectional parity with registered factories, so the constant must land with
the factory (Task 11). SqlDriver.DriverTypeName carries the string until then.
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
Code-review Minor: the record typed ParameterNames/Parameters/Members/
SelectedColumns as IReadOnlyList<T> but was handed the planner's live List<T>
instances. SqlQueryPlan documents itself as safe to hold for plan caching, so
an aliased list is a real hazard once the reader starts reusing plans across
polls: IReadOnlyList<T> is downcastable, and one mutation would corrupt every
subsequent poll served by that plan.
Copying at the record boundary fixes it for every call site at once rather
than relying on each planner branch remembering to call AsReadOnly.
Pinned by a test that mutates the caller's lists after construction; verified
load-bearing (reverting the record turns it red).
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
Executes a poll pass: resolve refs -> SqlGroupPlanner.Plan -> one command per
group -> slice the result set back to per-tag DataValueSnapshots, positionally
aligned with the caller's reference list (design §3.2).
The frozen-peer contract (design §8.3, the R2-01 S7 lesson) is the core of it.
CommandTimeout is only the server-side backstop; the client-side bound is a
linked CancelAfter PLUS Task.WaitAsync, because a linked token bounds only a
provider that honours it and ADO.NET providers vary. The whole per-group
operation — waiting for a concurrency slot, OpenAsync, and the query — runs off
one operationTimeout budget, so ReadAsync returns on time regardless of what the
database is doing. A breach Bad-codes that group (BadTimeout); caller
cancellation propagates instead, since nobody consumes a torn-down poll.
Also: absent row (BadNoData) stays distinct from a NULL cell (Uncertain, or Bad
under nullIsBad); duplicate plan members are all fed (two tags on one keyValue
bind one parameter but stay two members); the concurrency slot is released by the
work, not the waiter, so a frozen database can never hold more than
maxConcurrentGroups connections; the whole call throws only when the connection
itself cannot open, which is what earns PollGroupEngine's backoff.
SqlStatusCodes is driver-local, matching every sibling driver's own table — there
is no shared helper in Core.Abstractions and drivers do not reference the OPC UA
SDK. Values were read off Opc.Ua.StatusCodes rather than copied from a sibling,
because two of those tables carry transcription errors.
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
Three findings from the code review of the dialect seam:
- QuoteIdentifier used char.IsControl, which only covers Unicode category Cc.
Bidi overrides, zero-width spaces, soft hyphen and BOM are category Cf and
passed through untouched. They cannot escape the brackets, so this is not an
injection bypass — but the method rejects control characters precisely to
protect the integrity of logged/audited statements, and Cf characters spoof
rendered text while comparing byte-different from the real catalog name
(Trojan Source, CVE-2021-42574). Same rule, same rationale, wider category.
- The XML docs on both files asserted that identifiers 'are sourced only from
catalog-validated (INFORMATION_SCHEMA) names' and that rejection is 'a
backstop, not the primary defence'. Neither is true yet: design §8.1 does
specify that gate, but nothing implements it and no task in the plan
schedules one, so an authored TagConfig table/column reaches QuoteIdentifier
unfiltered. The docs now say so, because under-scrutinising this method on
the strength of a filter that does not exist is exactly the failure mode.
- The control-character test only pinned NUL, so a regression to a
Contains('\0') check would still have passed. Pinned the whole Cc category
plus the new Cf rule; verified load-bearing by deleting the guard and
observing exactly the 5 new cases go red.
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
The dialect owns the two things ADO.NET's System.Data.Common base types
cannot abstract: identifier quoting and the metadata-catalog SQL. It is
the driver's SQL-injection boundary — values always bind as DbParameters,
identifiers cannot, so the few that reach a command text are catalog-
sourced and pass through QuoteIdentifier.
QuoteIdentifier brackets one identifier part and doubles embedded ] (the
only metacharacter that can terminate a bracketed identifier early);
rejects null/empty/whitespace, >128 chars (T-SQL sysname ceiling), and any
control character (incl. NUL) as ArgumentException, without echoing the
untrusted value into the message. MapColumnType folds the design §3.7
families case-insensitively and never throws — an unrecognised family
falls back to String so a browse over an exotic column still renders;
timestamp/rowversion and time are deliberately NOT mapped to DateTime.
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
Joseph Doherty