Two follow-ups from the T13/T14 code review:
- M1: Add CachedWrite_StampsSourceNode_OnSubmitTelemetryRow and
CachedWrite_NoSourceNodeWired_LeavesSourceNodeNull to DatabaseCachedWriteEmissionTests,
mirroring the existing ApiOutbound SourceNode tests in
ExternalSystemCachedCallEmissionTests. Site-emitter coverage now symmetric
across both cached-call channels.
- M2: Clarify the GetService(INodeIdentityProvider) DI comments on the
CachedCallTelemetryForwarder and CachedCallLifecycleBridge factories:
it's test composition roots that may not register the provider, not
central production. Both site and central hosts always register it via
SiteServiceRegistration.BindSharedOptions.
Site: site emitters of SiteCallOperational (ExternalSystemClient, the script-API
cached call path in ScriptRuntimeContext, CachedCallLifecycleBridge) inject
INodeIdentityProvider and stamp SourceNode = NodeName at construction.
OperationTrackingStore call site in CachedCallTelemetryForwarder now stamps
SourceNode too.
Central: SiteCallAuditRepository.UpsertAsync INSERT includes SourceNode in the
column list; conditional monotonic UPDATE uses
COALESCE(@SourceNode, SourceNode) so later packets cannot blank a previously-
stamped value. After this commit every SiteCalls row carries node-a/node-b in
SourceNode (subject to monotonic preservation).
Site: inject INodeIdentityProvider where NotificationSubmit is built; stamp
SourceNode = NodeName at construction.
Central: NotificationOutboxActor.HandleSubmit copies submit.SourceNode onto
the Notification row; the repository INSERT persists it (EF tracked-entity
insert flows it through automatically; raw-SQL extension if not).
After this commit, every Notifications row carries the originating site
node-a/node-b in SourceNode. Existing notifications submitted pre-feature
remain NULL.
Two follow-ups flagged by code review on Tasks 11/12:
- Lock the back-compat contract for CentralAuditWriter's optional
`nodeIdentity = null` ctor parameter with two explicit tests
(`WriteAsync_PassesThroughCallerSourceNode_WhenNoProviderInjected` and
`WriteAsync_LeavesSourceNodeNull_WhenNoProviderInjected`). The previous
null-provider path was only exercised incidentally via legacy
CentralAuditWriterTests setups; the new tests make the contract explicit
and distinct from the "provider supplied, returns null" path.
- Document why the catch-block log references `evt` rather than the
post-stamp record: the three logged fields (EventId, Kind, Status) are
immutable across the filter+stamp chain, so referencing either name is
equivalent — but the comment warns future maintainers to switch names if
they ever add a field the chain mutates (e.g. SourceNode).
CentralAuditWriter injects INodeIdentityProvider and stamps the event before
handing to the repository. AuditLogRepository.InsertIfNotExistsAsync now
includes SourceNode in the INSERT column list. Caller-provided value wins
(supports any future direct-write callsite that already has its own node id).
Caller-provided SourceNode wins (preserves reconciled rows from other nodes);
otherwise the writer fills it from the local INodeIdentityProvider.NodeName.
Reads from the provider on every write — singleton lifetime means zero overhead.
Tidies flagged by code review on the T6/T7/T8 migration bundle:
- Add `.IsUnicode(false)` to the three SourceNode EF property mappings to
match every other ASCII varchar column on the same entities. Physical
column was already `varchar(64)` because `HasColumnType` wins, but the EF
model metadata flag was inconsistent.
- Add `unicode: false` to the three AddColumn<string> calls in the migrations
+ their Designer snapshots so the historical snapshots match the model.
- Update the model snapshot to carry IsUnicode(false) on each SourceNode entry.
- Document the SELECT-list invariant on SiteCallAuditRepository.QueryAsync:
EF Core's FromSqlInterpolated requires every entity-tracked column in the
result set, so future SiteCall columns must extend the list too.
- Amend plan Task 6 Step 2 to document the partition-aligned raw-SQL index
recipe and the staging-table sync requirement.
- AuditEventDto field 22, SiteCallOperationalDto field 12. Both follow the
existing empty-string-means-null convention.
- Mappers carry SourceNode end-to-end; round-trip tests cover both populated
and null cases.
- NodeName: semantic role-within-cluster identifier (node-a/node-b on sites,
central-a/central-b on central). Bound from ScadaLink:Node:NodeName.
- INodeIdentityProvider exposes the trimmed name (null if unconfigured) so
downstream audit writers can stamp the new SourceNode column.
- Adds SourceNode varchar(64) NULL to AuditLog, Notifications, and SiteCalls
tables with role-name semantics: node-a/node-b for site rows (qualified by
SourceSiteId), central-a/central-b for central direct-write rows.
- New IX_AuditLog_Node_Occurred (SourceNode, OccurredAtUtc) index.
- Reframes CLAUDE.md from documentation-only to implementation project.
- Adds docs/plans/2026-05-23-audit-source-node.md + tasks.json companion.
Inbound API audit rows (Channel = ApiInbound) now capture request and
response bodies in full up to a configurable 1 MiB per-body ceiling
(AuditLog:InboundMaxBytes), instead of the global 8 KiB / 64 KiB caps
that other audit channels use. Implements the M5-deferred response-body
capture in AuditWriteMiddleware via a write-only Stream wrapper that
forwards every byte to the framework's response sink while bounding the
audit copy at the capture site (ArrayPool-backed for the request side).
Other channels untouched.
The design doc claimed (in two places) that InboundAuthFailure rows
were excluded from the inbound full-body carve-out — but the actual
implementation gates the carve-out on Channel == ApiInbound, NOT Kind.
Every audit row the InboundAPI middleware emits (whether
Kind = InboundRequest or Kind = InboundAuthFailure) carries
Channel = ApiInbound, so both Kinds receive the inbound ceiling. That
is the intended behaviour: an auth-failure row's request body is
exactly the body the operator wants to see in full when investigating
a rejected request.
Update both occurrences (Decision block + Not in Scope block) to say
the carve-out applies to all Channel = ApiInbound rows regardless of
Kind. Pure documentation change — no code drift.
AuditWriteMiddleware previously buffered the FULL request and response
bodies into memory and only let DefaultAuditPayloadFilter trim them
after persistence. A 500 MiB upload allocated 500 MiB of MemoryStream
plus 1 GiB of UTF-16 string transiently before the filter pulled it
back to the 1 MiB inbound ceiling — the cap was real on the persisted
row but not at the capture site.
Inject IOptionsMonitor<AuditLogOptions> and read InboundMaxBytes
per-request (same convention as DefaultAuditPayloadFilter so a live
config change picks up the next request). The request reader now pulls
at most cap + 1 bytes into a UTF-8 byte-safe-truncated string and
rewinds the stream so the endpoint handler still sees the full body.
The response wrap is a new CapturedResponseStream that forwards every
Write / WriteAsync to the real sink (the client still receives all
bytes) while capturing at most cap + 1 bytes for the audit copy. The
middleware now sets PayloadTruncated itself when either body hit the
cap; the filter still OR's its own determination on top.
Adds a project reference from ScadaLink.InboundAPI to
ScadaLink.AuditLog so AuditLogOptions resolves. AuditLog does NOT
reference InboundAPI back, so no cycle is introduced.
Tests:
- All 21 existing AuditWriteMiddlewareTests still pass (the helper
gains an optional AuditLogOptions argument; default is the standard
1 MiB ceiling so existing small-body tests are unaffected).
- MiddlewareOrderTests' construction site updated for the new ctor
arg; a StaticAuditLogOptionsMonitor file-local double mirrors the
InboundChannelCapTests pattern.
- New RequestBody_AboveInboundMaxBytes_TruncatedToCap_PayloadTruncatedTrue
pins a 4 KiB cap against a 20 KB body: audit copy <= 4 KiB,
PayloadTruncated = true, downstream handler reads the full 20 KB.
- New ResponseBody_AboveInboundMaxBytes_TruncatedToCap_ClientStillReceivesAllBytes_PayloadTruncatedTrue
pins the same shape on the response side: client sink receives
20 KB, audit copy <= 4 KiB, PayloadTruncated = true.
InboundAPI test count: 133 -> 135.
Plan companion to the 2026-05-23 design doc. Seven tasks (#0 prep, #1-3
implementation TDD, #4-5 doc updates, #6 final sweep). Tracks via
.tasks.json for resumability.
Carve-out from Payload Capture Policy: ApiInbound rows capture
RequestSummary and ResponseSummary in full up to a configurable 1 MB
per-body ceiling (AuditLog:InboundMaxBytes), instead of the global 8 KB /
64 KB caps. No schema change; existing redaction (headers + per-target
body redactors) still applies before persistence.
ResizeHandle_DraggingWidensColumn_AndSurvivesReload called page.ReloadAsync()
immediately after the resize drag, racing the asynchronous persist: pointer-up
fires a fire-and-forget JS→.NET OnColumnResized invoke that round-trips back
through JS interop to write sessionStorage. When the reload won the race the
restored grid fell back to the default column width and the test failed
(~1 in 3 runs).
Wait for auditGrid:columnWidths to land via the existing WaitForStorageKeyAsync
helper before reloading — the same guard the sibling
ColumnOrderAndWidths_PersistAcrossReload_ViaSessionStorage test already uses.
Verified: 6/6 consecutive passes.
Make the seven sidebar section groups (Admin, Design, Deployment,
Notifications, Site Calls, Monitoring, Audit) collapsible. New NavSection
component renders a header toggle button (chevron) and reveals its items
only while expanded; NavMenu owns the expanded-section set.
Behaviour: sections are collapsed by default; state persists in the
`scadabridge_nav` cookie (written/read via the new nav-state.js JS interop,
mirroring treeview-storage.js) so it survives reloads and reconnects;
navigating into a section auto-expands it and remembers it. The Dashboard
item stays sectionless and always visible.
Tests: NavMenu bUnit tests expand sections before asserting items and add
collapsed-by-default / toggle / cookie-persistence cases; Playwright nav
tests expand sections before clicking links; new NavCollapseTests covers
the feature E2E. Build 0 warnings; bUnit 545 passed; Playwright nav suite
green (the unrelated AuditGridColumnTests resize-reload case remains
pre-existing flaky — an un-awaited save race in that test).
Rename the user-facing product name from ScadaLink to ScadaBridge across
the six display strings (browser title, sidebar brand, login + not-authorized
headings, dashboard welcome/subtitle). Namespaces, assemblies, config keys,
and _content/ScadaLink.CentralUI asset routes are unchanged.
Apply the technical-light design system: vendor theme.css + IBM Plex fonts
into the CentralUI RCL, include theme.css globally (after Bootstrap so its
--bs-* token overrides win), and restyle the layout chrome to a light
sidebar — white surface, hairline rules, ink text, accent-blue active item,
the brand accent mark. Page markup stays Bootstrap and inherits the warm
paper background, Plex type, accent, and hairline borders via the tokens.
Tests: build 0 warnings; bUnit 542 passed; Playwright 64 passed.
Code-review follow-ups on the same-page drill-in fix (3f1c0e5):
- Wrap HandleLocationChanged's body in InvokeAsync — LocationChanged can
fire off the renderer's synchronization context.
- Document that a paramless /audit/log navigation intentionally preserves
the last applied filter (drill-ins always carry query params).
The drilldown drawer's 'View this/parent execution' actions call
NavigationManager.NavigateTo('/audit/log?executionId=...') while the
user is already on the routed AuditLogPage. Blazor treats this as a
same-component navigation, so OnInitialized does not re-run and
ApplyQueryStringFilters() (which was wired only to OnInitialized) never
re-parsed the new query string: _currentFilter stayed stale and the
results grid never reloaded to the drill-in target.
AuditLogPage now subscribes to NavigationManager.LocationChanged,
re-applies the query-string filters on every location change (closing
the drawer and calling StateHasChanged), and unsubscribes via
IDisposable. The 'View parent execution' drill-in now genuinely lands
on /audit/log?executionId={parentId} with the grid reloaded.
Also corrects the Playwright test wait: a same-page query-string Blazor
navigation pushes history.pushState over the SignalR circuit rather
than triggering a document load, so WaitForLoadState(NetworkIdle)
returned before the URL settled. Switched to WaitForURLAsync, the
correct primitive for SPA/pushState navigations.
- AddColumnIfMissing is now shared by ExecutionId and ParentExecutionId;
drop the ExecutionId-specific tag.
- AuditLogRepository.GetExecutionTreeAsync doc no longer hardcodes the
MAXRECURSION literal; reference the ExecutionChainMaxDepth const instead.
The headline ParentExecutionIdCorrelationTests intermittently failed under
full-suite parallel load, seeing 6 of 7 routed-run rows (NotifySend missing).
Root cause: WaitForSiteRowsPersistedAsync checked only a row *count*, which a
partial snapshot could satisfy before the last-emitted NotifySend row settled,
letting the SiteAuditTelemetryActor drain a partial batch. Fix is test-only:
wait on the specific audit Kinds (guaranteeing NotifySend is durably in SQLite
before the assertion) and widen the assertion ceiling 30s -> 90s for drain
headroom under load. Also drops leftover // DIAG sampler debug scaffolding.
