From 02d7c2428b50d3b97dda1046b98f863302662d74 Mon Sep 17 00:00:00 2001 From: Laurance Walden <6620582+lwalden@users.noreply.github.com> Date: Wed, 10 Jun 2026 14:49:44 -0700 Subject: [PATCH 1/2] fix(host): align ApplicationInsights packages so the isolated worker boots (S5-004r) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Microsoft.ApplicationInsights.WorkerService 3.0.0 is OTel-based and removed classic API types (ITelemetryInitializer) that Functions.Worker.ApplicationInsights 2.50.0 is built against, so the worker died at DI bootstrap with TypeLoadException. Downgrade to 2.23.0 (latest classic 2.x) — both packages are used by Program.cs, so removal was not an option. Also: - Drop the OpenTelemetry.Api 1.15.3 pin: the vulnerable chain (GHSA-g94r-2vxg-569j) came via AI 3.0.0 -> Azure.Monitor.OpenTelemetry.Exporter and is gone on 2.23.0; `dotnet list package --vulnerable --include-transitive` is clean without it. - Register bare TacticalConfig in the composition root: OptionsSleeveManager takes the bare type (siblings take IOptions), which failed DI graph validation on boot — a second boot-blocker previously masked by the TypeLoadException and invisible to the unit suite (tests never build the host graph). - Runbook preflight: add a host-boot smoke line (function banner + no TypeLoadException proves the worker process actually loaded). Verified by booting the real host: Azurite + func start, worker initialized, both DailyOrchestrator timers listed, stable 35s+, zero crash signatures. 590/590 tests green. Co-Authored-By: Claude Fable 5 --- docs/paper-validation-runbook.md | 5 +++++ src/TradingSystem.Functions/Program.cs | 6 ++++++ .../TradingSystem.Functions.csproj | 13 +++++-------- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/docs/paper-validation-runbook.md b/docs/paper-validation-runbook.md index e375caf..92b8e84 100644 --- a/docs/paper-validation-runbook.md +++ b/docs/paper-validation-runbook.md @@ -46,6 +46,11 @@ Run this check each trading morning **before 5:00 AM PT in winter (PST) / 6:00 A 4. **Startup-log smoke check:** the worker banner lists both timer functions — `DailyOrchestrator_PreMarket` and `DailyOrchestrator_EndOfDay`. If either is missing, the build is broken or the wrong project started; do not assume the day will run. +5. **Host boot smoke:** after `func start`, the function-list banner must appear with **no + `TypeLoadException`** (or repeated "worker process" restarts) in the startup output. + The banner proves the isolated worker process actually loaded its DI graph — a + package-version mismatch (S5-004r) can pass the full unit suite yet crash the worker + at bootstrap, because tests never load the Functions host. ## 3. Timer schedule diff --git a/src/TradingSystem.Functions/Program.cs b/src/TradingSystem.Functions/Program.cs index 65bf616..c4cfd7d 100644 --- a/src/TradingSystem.Functions/Program.cs +++ b/src/TradingSystem.Functions/Program.cs @@ -40,6 +40,12 @@ context.Configuration.GetSection("TradingSystem")); services.Configure( context.Configuration.GetSection("TradingSystem:Tactical")); + // S5-004r: OptionsSleeveManager takes the bare TacticalConfig (its siblings take + // IOptions), so the bare type must be resolvable or the worker dies + // at DI validation on boot. This was masked by the AI-package TypeLoadException — + // the unit suite never builds the full host graph, only the real `func start` does. + services.AddSingleton(sp => + sp.GetRequiredService>().Value); services.Configure( context.Configuration.GetSection("IBKR")); services.Configure( diff --git a/src/TradingSystem.Functions/TradingSystem.Functions.csproj b/src/TradingSystem.Functions/TradingSystem.Functions.csproj index 39bd8b9..00a3ef8 100644 --- a/src/TradingSystem.Functions/TradingSystem.Functions.csproj +++ b/src/TradingSystem.Functions/TradingSystem.Functions.csproj @@ -17,17 +17,14 @@ - + + - - From 364c9eb0797ed8cb63f645ea1e500141144e391f Mon Sep 17 00:00:00 2001 From: Laurance Walden <6620582+lwalden@users.noreply.github.com> Date: Wed, 10 Jun 2026 15:13:05 -0700 Subject: [PATCH 2/2] docs(review): App Insights gates, runbook preflight order, debt entries (S5-004r) Judge fix-now items (all docs/comment-only, no behavior change): - Program.cs: document the App Insights enablement gate (registration is inert until APPLICATIONINSIGHTS_CONNECTION_STRING is set) and clarify that the HttpClient ILogger filter does not cover AI 2.x dependency telemetry. - Runbook: host-boot smoke now precedes the function-list check (steps 4-5) -- the banner is meaningless until the worker has provably booted. - Functions csproj: trace why the S5-005 OpenTelemetry.Api pin was removed (vulnerable chain only existed on the AI 3.0.0 dependency tree). - DECISIONS.md Known Debt: KD-005 (AI 2.x->3.x upgrade blocked on Functions Worker AI compatibility) and KD-006 (discord.com URI redaction required before ever enabling App Insights). Co-Authored-By: Claude Fable 5 --- DECISIONS.md | 2 ++ docs/paper-validation-runbook.md | 11 ++++++----- src/TradingSystem.Functions/Program.cs | 17 ++++++++++++++--- .../TradingSystem.Functions.csproj | 7 +++++++ 4 files changed, 29 insertions(+), 8 deletions(-) diff --git a/DECISIONS.md b/DECISIONS.md index 5527c87..50420e2 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -346,6 +346,8 @@ Select which sleeve(s) activate first and final initial capital split/account ma | KD-002 | Claude API key not provisioned — NOT required under default posture: ADR-029/030 gateway-or-rules with `DirectApiFallbackEnabled=false` means zero metered spend (gateway bearer key provisioned via S5-004). Becomes relevant only if the owner explicitly enables the metered fallback | Conditional — regime integration shipped, degrades to deterministic rules | 2026-02-16 | | KD-003 | Backtest pipeline paths use Python (not .NET) | Must install Python + deps separately for backtesting | 2026-04-07 | | KD-004 | ✅ RESOLVED 2026-05-29 (S3-001, PR #70). ~~CachingMarketDataService.cs ~360 lines, exceeds 300-line architecture-fitness threshold (grew across S2-001/003/005/006)~~ — decomposed into a 125-line facade + new 280-line MarketRegimeProvider (internal composition, no DI change, zero behavior change) per ADR-017 | Maintainability — closed | 2026-05-29 | +| KD-005 | **ApplicationInsights 2.x→3.x upgrade gate (S5-004r).** `Microsoft.ApplicationInsights.WorkerService` is pinned to the classic 2.x line (2.23.0): AI 3.x is OTel-based and removed classic API types (`ITelemetryInitializer`) that `Microsoft.Azure.Functions.Worker.ApplicationInsights` 2.50.0 is compiled against — pairing them crashes the isolated worker at DI bootstrap with `TypeLoadException`. Upgrade is BLOCKED until the Functions Worker AI package ships a 3.x-compatible release; when unblocking, re-evaluate the OpenTelemetry.Api pin removed in S5-004r (see comment in `TradingSystem.Functions.csproj`) and clear KD-006 in the same change | Stuck on classic 2.x telemetry SDK; no OTel-native pipeline. Boot-blocking if upgraded prematurely | 2026-06-10 | +| KD-006 | **discord.com URI-redaction gate before enabling App Insights (S5-004r).** AI 2.x *dependency* telemetry records the full request URL of outbound HTTP calls — for the Discord named clients that URL IS the token-bearing webhook URL. This path bypasses the `System.Net.Http.HttpClient` ILogger filter in `Program.cs` (that filter only covers log telemetry) and is dormant solely because `APPLICATIONINSIGHTS_CONNECTION_STRING` is unset everywhere. Do NOT set that variable until a telemetry processor/initializer that redacts or drops discord.com dependency URLs is in place (or AI 3.x with OTel redaction supersedes it — KD-005) | Secret-leak risk into App Insights if telemetry is ever enabled without redaction | 2026-06-10 | --- diff --git a/docs/paper-validation-runbook.md b/docs/paper-validation-runbook.md index 92b8e84..d3a9320 100644 --- a/docs/paper-validation-runbook.md +++ b/docs/paper-validation-runbook.md @@ -43,14 +43,15 @@ Run this check each trading morning **before 5:00 AM PT in winter (PST) / 6:00 A classification (ADR-029/ADR-030) — **this is expected behavior, not an incident**. Restart the gateway (`D:\Source\claude-gateway`) when convenient. 3. **Functions worker** started (`func start` per section 1). -4. **Startup-log smoke check:** the worker banner lists both timer functions — - `DailyOrchestrator_PreMarket` and `DailyOrchestrator_EndOfDay`. If either is missing, - the build is broken or the wrong project started; do not assume the day will run. -5. **Host boot smoke:** after `func start`, the function-list banner must appear with **no +4. **Host boot smoke:** after `func start`, the function-list banner must appear with **no `TypeLoadException`** (or repeated "worker process" restarts) in the startup output. The banner proves the isolated worker process actually loaded its DI graph — a package-version mismatch (S5-004r) can pass the full unit suite yet crash the worker - at bootstrap, because tests never load the Functions host. + at bootstrap, because tests never load the Functions host. Run this check first: the + function list in step 5 is meaningless until the worker has provably booted. +5. **Startup-log smoke check:** the worker banner lists both timer functions — + `DailyOrchestrator_PreMarket` and `DailyOrchestrator_EndOfDay`. If either is missing, + the build is broken or the wrong project started; do not assume the day will run. ## 3. Timer schedule diff --git a/src/TradingSystem.Functions/Program.cs b/src/TradingSystem.Functions/Program.cs index c4cfd7d..f84f349 100644 --- a/src/TradingSystem.Functions/Program.cs +++ b/src/TradingSystem.Functions/Program.cs @@ -59,7 +59,12 @@ services.Configure( context.Configuration.GetSection("Reporting")); - // Application Insights + // Application Insights — registration is inert until APPLICATIONINSIGHTS_CONNECTION_STRING + // is set (it is NOT set in local.settings.json or any deployed config today): with no + // connection string the SDK builds a disabled TelemetryConfiguration and sends nothing. + // Before ever setting that variable, the discord.com URI-redaction gate in DECISIONS.md + // (Known Debt KD-005/KD-006) must be satisfied — AI 2.x dependency telemetry records full + // request URLs, which for the Discord named clients are token-bearing webhook URLs. services.AddApplicationInsightsTelemetryWorkerService(); services.ConfigureFunctionsApplicationInsights(); @@ -130,8 +135,14 @@ builder.SetMinimumLevel(LogLevel.Information); // S5-004 (review): IHttpClientFactory's LogicalHandler logs the full request URI at // Information on every send — for Discord named clients that URI IS the token-bearing - // webhook URL. Warning+ only for the HttpClient categories keeps the secret out of - // console/App Insights logs while preserving error visibility. + // webhook URL. Warning+ for every "System.Net.Http.HttpClient.*" ILogger category + // keeps the secret out of console logs and out of App Insights *log* telemetry while + // preserving error visibility. Scope note (S5-004r review): this filter governs only + // ILogger output — it does NOT cover App Insights *dependency* telemetry, which the + // AI 2.x SDK emits with the full request URL outside the logging pipeline. That path + // is dormant solely because APPLICATIONINSIGHTS_CONNECTION_STRING is unset (see the + // enablement-gate comment at the App Insights registration above and DECISIONS.md + // Known Debt). builder.AddFilter("System.Net.Http.HttpClient", LogLevel.Warning); }); }) diff --git a/src/TradingSystem.Functions/TradingSystem.Functions.csproj b/src/TradingSystem.Functions/TradingSystem.Functions.csproj index 00a3ef8..d30cb75 100644 --- a/src/TradingSystem.Functions/TradingSystem.Functions.csproj +++ b/src/TradingSystem.Functions/TradingSystem.Functions.csproj @@ -25,6 +25,13 @@ +