# Project: Microsoft Agentic Harness

## Purpose
Production-grade template for a Microsoft Agent Framework agent with a full agentic harness — multi-skill agents, local plugins, MCP, tools, RAG, and knowledge graph systems — modeled after Claude Code's architecture. Built on the ApplicationTemplate Clean Architecture pattern. Designed for enterprise consumers to clone and extend.

## Stack
- C# .NET 10, Clean Architecture, CQRS/MediatR, FluentValidation, AutoMapper
- Microsoft.Agents.AI, Microsoft.Extensions.AI, OpenAI SDK (Azure OpenAI runs on its `/openai/v1/` route)
- MCP (Model Context Protocol) server/client — HTTP transport with JWT auth
- EF Core with SQLite (plan state persistence), IDbContextFactory for short-lived contexts
- Docker.DotNet (container sandbox)
- RAG: Azure AI Search, FAISS, SQLite FTS5, ManagedCode.GraphRag
- Knowledge Graph: Neo4j, Kuzu, PostgreSQL, Leiden community detection
- Governance: Polly (resilience), EWMA drift detection, JSONL audit stores
- OpenTelemetry (Grafana + Tempo + Prometheus + Azure Monitor)
- xUnit, Moq, coverlet

## Architecture
Clean Architecture with Domain → Application → Infrastructure → Presentation layers.
Reference implementation: `C:\CodeRepos\ApplicationTemplate` (same layer structure, DI patterns, and conventions).

Key architectural concepts from the reference:
- **Skills System**: Multi-skill agents with prerequisite ordering, dual mode (Managed/Injected), plugin-sourced skills
- **Plugin System**: Local plugin declarations with manifest reading, skill + MCP server wiring, boundary governance (AllowedTools/DeniedTools/AutonomyLevel)
- **Tool Output Compression**: MediatR pipeline behavior with content-type detection and strategy-specific compression
- **Keyed DI Tools**: Tools registered with string keys (`"file_system"`, `"calculation_engine"`) for lazy resolution from skill declarations
- **Agent Manifest (AGENT.md)**: Declarative agent config with tool declarations, state config, decision frameworks
- **MCP Server**: ASP.NET Core WebAPI exposing tools/prompts/resources via MCP protocol
- **Factory Pattern**: AgentFactory, ChatClientFactory, AgentExecutionContextFactory for consistent agent construction
- **MediatR Pipeline**: validation, authorization, idempotency, caching, tracing and timeout from `Application.Common`, then the AI behaviors (unhandled-exception safety net, content safety, token budget, tool output compression and others) from `Application.AI.Common`, in registration order
- **DAG Plan Execution**: PlanExecutor orchestrates PlanGraph with bounded concurrency, checkpoint/resume via EfCorePlanStateStore, error recovery (retry/escalate/skip)
- **Sandbox Isolation**: ProcessSandboxExecutor (Job Objects) and DockerSandboxExecutor with HMAC attestation. Closed-by-default capability model
- **Step Executors**: Keyed DI by StepType enum — LlmCall, ToolUse, HumanGate, ConditionalBranch, SubPlanInvocation
- **Partial Class Pattern**: Large files split into partials by responsibility (PlanExecutor.Scheduling.cs, PlanExecutor.Recovery.cs, etc.)
- **Skill Training Loop**: `TrainSkillCommandHandler` chains the 6 stages on the same call stack (no MediatR re-entrance inner-loop); epoch-boundary mechanisms (SlowUpdate, MetaSkillUpdate) are separate CQRS commands dispatched via `IMediator` so they get the standard pipeline (validation, audit, telemetry)

## Documentation
- **Developer Onboarding Guide**: `documentation/onboarding/` — 18-chapter guide for engineers (deployed at `/`). Chapter 17 is the Execution API integration guide (bundles, workflows, tool discovery); its OpenAPI spec is `documentation/onboarding/assets/openapi/bundle-api.yaml` (filename kept deliberately — it is a published URL) (hand-written; to change the contract, follow *Change the wire contract* in `src/Content/Presentation/Presentation.ExecutionApi/README.md`)
- **Architecture Guide**: `documentation/architecture/` — Azure infrastructure playbook (deployed at `/architecture/`)
- **Security Guide**: `documentation/security/` — threat model, governance, sandbox, egress/SSRF, content safety, OWASP Agentic evals (deployed at `/security/`)
- **Interactive Course**: `documentation/agentic-harness-course/` — Visual course for non-technical audiences (deployed at `/agentic-harness-course/`)
- **Reference Catalogue**: `documentation/reference/` — patterns & technologies reference (deployed at `/reference/`)
- Additional design specs live in `documentation/design/` and `documentation/blueprints/` (markdown, not deployed)
- **Capability Showcase**: `documentation/showcase/` — interactive architecture showcase (deployed at `/showcase/`)
- GitHub Pages workflow: `.github/workflows/pages.yml` — deploys all six sites on push to main

## Commands
- `dotnet build src/AgenticHarness.slnx` — Build
- `dotnet test src/AgenticHarness.slnx` — Run all tests
- `dotnet test --collect:"XPlat Code Coverage"` — Tests with coverage
- `dotnet run --project src/Content/Presentation/Presentation.ConsoleUI` — Run console

## Verification
After changes: `dotnet build src/AgenticHarness.slnx && dotnet test src/AgenticHarness.slnx`

## Code Style
- Immutability: records, `with` expressions, `ImmutableList<T>`, init-only properties
- PascalCase (classes/methods/props), `_camelCase` (private fields), camelCase (locals/params)
- Functions <50 lines, no nesting >4 levels
- Result<T> pattern for error handling, structured logging (no console.log)
- FluentValidation on all DTOs, validate at system boundaries

## Task Approach
1. Check reference implementation at `C:\CodeRepos\ApplicationTemplate` for existing patterns before creating new abstractions
2. Present options when trade-offs exist between agent framework approaches
3. Implement in layers: Domain models first, Application interfaces, Infrastructure implementations, Presentation last
4. Run build + tests after each meaningful change
5. Flag anything that diverges from the ApplicationTemplate patterns

## Quality Bar

This is a production template that enterprise consumers will clone. Corners cut now propagate to every downstream consumer. The global "Optimize for Best Outcome, Not Speed" rule applies with extra weight here:

- Match or exceed the reference implementation (`C:\CodeRepos\ApplicationTemplate`) on patterns, abstractions, and rigor. Never ship something here that you'd reject when reviewing the reference.
- When the reference is silent on a problem, design the best answer; do not invent a smaller answer just because the reference didn't address it.
- Every public type ships with full XML docs (already a rule) — same reasoning. Consumers are reading this as teaching material.

## Common Mistakes
- Creating new abstractions when ApplicationTemplate already has one: check `Application.AI.Common/Interfaces/` first
- Registering tools without keyed DI: always use `AddKeyedSingleton<T>(toolName, ...)` pattern
- Forgetting MediatR pipeline behaviors when adding new commands: register in `DependencyInjection.cs`
- Hardcoding AI model config: use `AppConfig.AI.AgentFramework` section, never inline
- Skipping content safety middleware in agent factory: always wire through `AgentFactory`
- Creating step executors without registering them as keyed services by StepType
- Using SandboxOptions (Domain config) when you need SandboxExecutionOptions (Application container config) — they're different classes
- Manually incrementing entity Version — SqliteVersionInterceptor handles this on save
- Forgetting to check Result.IsSuccess on state store operations
- Adding NotifyStepStarted in step executors — PlanExecutor owns notification centrally
- Using `PluginSource` on a skill without declaring the plugin in `PluginsConfig`: the plugin won't load and tools won't resolve
- Forgetting that DeniedTools on a plugin are bypass-immune: they can't be overridden by auto-approve modes
- Using `Replace` with empty `Content` to remove text in a `Patch` — `PatchApplier` rejects it as a failed edit; use `Delete` op explicitly so the audit trail captures intent
- Returning raw exception text in `Result.Fail` from skill-training handlers — must use stable scrubbed codes (`skill_training.*`) and log the full exception via structured logging; HTTP-backed proposers can leak SAS tokens in exception messages otherwise
- Persisting projected gate scores across checkpoint reloads — round-trip float→text→float can flip Accept/Reject by 1 ULP; orchestrator should re-project on each call via `IGateEvaluator.SelectGateScore`
- Forgetting the `NotConfiguredPatchProposer` / `NotConfiguredRolloutRunner` defaults — these throw on first use; a consumer that invokes `TrainSkillCommand` without registering real impls gets an `InvalidOperationException` at runtime, not a silent no-op
- **Implementing a subtractive or rewriting `AIContextProvider` on `ProvideAIContextAsync`.** That hook is contractually **additive**: the base merge computes `Tools = input.Concat(provided)` and `Instructions = input + provided`. A provider that filters, wraps, or replaces anything there is silently inert — every item it drops is restored from the input, and every item it keeps is published twice. To subtract or rewrite, override `InvokingCoreAsync`, call `base` first, then transform the result. A provider that only *adds* must return **just its own contribution**, never the input echoed back. This landed four separate times (`ToolPermissionFilter`, `GoverningToolContextProvider` — a security control that was dead — plus both recall providers). All four had green unit tests because those tests called the protected hook directly; **any test of an `AIContextProvider` must drive the public `InvokingAsync`**, which is what `AIContextProviderMergeContractTests` does for every provider in `Application.AI.Common`.
- **Re-implementing the conversation ownership check at a call site.** `IConversationStore` enforces it: every operation naming one conversation takes a `callerId` and throws `ConversationAccessDeniedException` for a record owned by anyone else. It did not always, and the comparison `record.UserId != callerId` ended up hand-written in **six** places across four files with three different failure shapes. Pass the authenticated caller through and let the store refuse. Two consequences: a **blank** `callerId` is an `ArgumentException`, never a wildcard; and a **mocked** store enforces nothing, so any test proving an intruder is refused must stub the throw explicitly (see `AgUiRunHandlerTests`, `ConversationOrchestratorTests`) — otherwise it passes while asserting nothing.
- **Deciding provider-failure behaviour from the .NET exception type.** Each provider SDK throws a different type for the same HTTP status — Azure OpenAI/OpenAI throw `ClientResultException`, Azure AI Inference throws `RequestFailedException`, Anthropic throws `HttpRequestException`. The old Polly `ShouldHandle` predicates listed types, so they matched **only Anthropic** — which is not in the shipped fallback chain, leaving retry and the circuit breaker inert for both providers that are. All three decisions (retry, breaker accounting, cross-provider fallback) now go through `IProviderErrorClassifier`; adding a provider means teaching the classifier its shape, never adding a type to a predicate. Two rules it encodes: a status that says *transient* (429/5xx) is **never** overridden by message text, because a false "fatal" both skips retries and halts the chain; and an unrecognised failure is `Unknown` — not retried, but still counted against provider health. Chain members are also built with the SDK's own retry **disabled** (`GetChatClientWithoutProviderRetryAsync`) so Polly is the single retry authority; the non-resilient path keeps SDK retry because nothing else wraps it.
- **Treating "no knowledge scope" as a safe default — it is not, it means GLOBAL.** `PlannerScopeFilter.VisibleTo` and `TenantIsolatedGraphStore` read a null owner as a world-readable record, so any path where identity resolution yields nothing silently publishes data. This defect has landed three separate times (host never mounted `KnowledgeScopeMiddleware`; the scope resolver rejected a token shape ownership accepted; an ambiguous claim resolved to null). The invariant: an **authenticated** request must either establish a scope or be **rejected** — never proceed unscoped. Only genuinely unauthenticated callers may run unscoped. Resolve identity solely through `ClaimsPrincipalExtensions.GetUserIdOrNull()`; never add a second precedence ladder.
- **Discriminating an outcome by a shared type's field that carries a different meaning depending on which code path produced it — the fix is a distinct type, not a better field.** Landed twice against `IacSandboxRunner`'s pre-dispatch refusal before the shape was retired for good (#421). First cut (#405/#406/#407): `ExitCode is null` looked like a clean signal for "the sandbox refused to dispatch before it ever ran," but `ProcessSandboxExecutor`/`DockerSandboxExecutor` also leave `ExitCode` null on a timeout, a reserved-env-grant rejection, and an egress-preflight block — every one of which genuinely dispatched and signed a failure attestation; silently mislabeled all three as governance denials, caught only on a second review pass. Correction: `Attestation is null`, the one field both executors leave unset in exactly the case that matters, centralized behind `WasRefusedBeforeDispatch`/`FailIfRefused`. Both fixes picked a better field; neither removed the underlying defect — a caller still had to remember to *call* the check, and the check itself could still drift to the wrong field again (the `plan`/`validate` split in that same PR's own history is exactly this: one dispatch site got the check a full commit after its sibling). #421's fix: `IacSandboxRunner.RunAsync` now returns `Result<SandboxExecutionResult>` and represents a refusal as `Result.Forbidden(...)` — a genuinely distinct outcome instead of a look-alike object with the same shape as a real one. That closes the specific defect this entry is about (a *shared field* silently meaning two different things), but it is not full compile-time enforcement: `Result<T>.Value` stays nullable regardless of `FailureType`, so every call site still does `dispatch.Value!` after checking the failure branch — a caller who skips that check still gets a null-forgiving operator the compiler accepts and a runtime `NullReferenceException`, not a compile error. The type distinction removes the "which field do I trust" ambiguity that caused this bug twice; it does not remove the need to remember to check at all. When a shared result type conflates two meanings, look first at whether the producer can return a different *type* for the different case, not just a more reliable field to check on the same one — and when adding any discriminator over a shared result type, check what every producer of that type actually sets on every path, not just the two cases in front of you.
- **Fixing one instance of a duplicated dispatch pattern and stopping there.** `TerraformGenerator.PlanAsync` runs two sequential CLI dispatches (`validate`, then `plan`); the governance-refusal guard above landed on `validate` only — `plan` needed the identical check (a hot-reloaded operator override can revoke access between the two calls, since `terraform plan` can run for minutes) and didn't get it until a second review pass noticed the new regression test only covered the first dispatch. When a fix targets one occurrence of a pattern repeated within the same method or across sibling files, grep for every other occurrence before calling the fix done — don't rely on a reviewer to find the sibling.
- **Adding a new enum member without updating every hand-maintained list that claims to mirror "all values of the enum."** `RedactionCategory.VendorApiKey` (PR #473) was added and correctly picked up by every `RedactionCategories.All`-style computed call site (`Enum.GetValues<RedactionCategory>()`), but `ContentCaptureConfig.RedactionCategories` and `LogsConfig.RedactionCategories` are separate hand-typed `List<string>` defaults — each documented in its own XML comment as "all categories from the enum" — that live in `Domain.Common` specifically so they don't have to reference `Domain.AI` (see the doc comments on both properties). Both silently stopped being true: a bare Slack/GitHub/OpenAI-shaped token would export in cleartext through the config-driven telemetry/log-redaction paths even though the new regex existed and worked everywhere else. Caught by CI's `correctness-review` gate, not by the two `/code-review` passes that preceded push. When a `RedactionCategory` (or any enum with a similar "default = every member" mirrored list) gains a member, grep for every hand-maintained string list of category names, not just the computed call sites — and add a test asserting the default list equals `Enum.GetNames<TEnum>()` so the next addition fails loudly instead of silently (see `ContentCaptureConfigValidatorTests.DefaultValues_MatchEveryRedactionCategoryEnumMember`).
- **Closing a governance check against the nearest intermediate filter instead of the value's true final, published state.** Landed twice in one PR (#476, `ToolChainBuilder`'s call-once tool registration). A plugin-sourced skill's `call-once-per-conversation` declaration was registering a tool into the process-global, never-pruned `IToolCallOncePolicy` before the plugin's `AllowedTools`/`DeniedTools` boundary had run — first fix moved registration to just *after* that boundary (`ApplyPluginBoundaryIfPluginSkill`), which closed the plugin-bypass CI's `security-review` gate found but was still one step too early: `FinalizeChain`'s `ReservedPlanCapabilityFilter` and, for any multi-skill agent, `ResolveSurvivingTools`' cross-skill first-party-precedence and drift/collision withholding both run *after* that point too — caught only by a second pass, from CI's `correctness-review` gate. A tool an untrusted MCP server declared call-once could still poison the policy for a same-named first-party tool that won the precedence fight, inverting the exact security property first-party precedence exists to provide. Correct fix: register only at a genuine whole-agent-set exit (here, `BuildToolsAsync`/`BuildMergedToolsWithSourcesAsync`), against the actual fully-filtered, fully-deduplicated surface a caller receives — never at an intermediate stage, however far downstream it looks, in a multi-stage resolution/filtering pipeline. When a fix moves a check "past" one filter, trace the value all the way to where it is actually handed to the caller before calling the fix done — a pipeline with N stages needs the check after stage N, not after whichever stage happens to be nearest the original bug.
- **Giving a wrapper class a real `ILogger<T>` dependency when it's constructed from inside the `ILoggerFactory` replacement factory that builds it.** #457's local-log-redaction wiring gave `ContentFilterLocalLogRedactor` (resolved by `IServiceCollectionExtensions.WrapWithLocalLogRedaction`'s `ILoggerFactory` replacement delegate) a normal `ILogger<ContentFilterLocalLogRedactor>` constructor parameter — which itself requires resolving `ILoggerFactory`, the very service still being constructed. .NET's container doesn't detect this shape of cycle (it's hidden behind a factory delegate, not a constructor graph it can walk), so instead of throwing it silently re-enters the factory forever: every host hangs at first `ILogger<T>` resolution. This was live for most of a working session before an independent review caught it — every `WebApplicationFactory`-based integration test hang in that window was misdiagnosed as machine flakiness rather than suspected as new code (see the "measure the control" standing rule — the actual control, a redactor variant with no `ILogger<T>` dependency, resolved in 62ms every time). Fix: `NullLogger<T>.Instance` as a field default, no `ILogger<T>` constructor parameter at all — the only thing that actually breaks the cycle, since anything that resolves through DI reaches back into the same factory. **Prevention, not just the fix:** the moment any change touches a core cross-cutting registration (`ILoggerFactory`, `IHttpClientFactory`, or anything the whole app depends on), run `Presentation.Common.Tests/Composition/ValidateOnBuildSweepTests` immediately — it builds the real container with `ValidateOnBuild = true`, which eagerly constructs every registered service and would have caught this in under a second instead of over an hour of misattributed test hangs.
- **Redacting/rewriting a shared object by reasoning about how its type "usually" behaves, instead of reading how this codebase's actual consumers use it.** Landed twice in the same method in one PR (#499, `RedactingLogger.BeginScope`, both caught by CI's `correctness-review`, not by two prior local passes). First cut: collapsed any unrecognized scope object to a redacted string via `ToString()` — broke `ExecutionScopeProvider.GetCurrentScope`'s `scope is ExecutionScope` type check, since a redacted string is no longer that type. Second cut: reconstructed a structured (`IEnumerable<KeyValuePair<string, object?>>`) scope as a plain `List<KeyValuePair<string, object?>>` — broke `ColorfulConsoleFormatter.WriteScopeInformation`, which renders a scope by calling `ToString()` on whatever was pushed; a compiler-generated templated `BeginScope("...{Arg}...", args)` call's state overrides `ToString()` to render the interpolated text, which the reconstructed `List<T>` cannot reproduce, so every templated scope's console output silently became useless boilerplate. Both fixes were reasoned from general knowledge of how .NET logging scopes are typically consumed; both were wrong for *this* codebase's actual formatters, discovered only after pushing. The eventual fix redacts only the one shape with zero type-identity risk (a bare string) and leaves everything else untouched, with the residual gap tracked rather than "fixed" a third time the same way. That gap was later closed by a genuinely different design (#500): a templated scope is rebuilt by the same framework factory that built it (`LoggerExtensions.BeginScope(format, args)`, with string arguments redacted), so its runtime type, its `ToString()` rendering and its enumerated keys all survive, and a test asserts the type is unchanged. Before changing the behavior of any object that flows through multiple consumers you don't fully control — a logging scope, a shared wrapper, a redaction pass-through — grep every real consumer in the codebase and read what each one actually does with it, before writing the fix. Do this once, thoroughly; don't let CI find it for you twice.
- **Shipping a control that nothing invokes — and writing documentation that says it is enforcing.** This has now landed **six** times, and the sixth (`ToolCallReplayConfigValidator`, PR #516) is the cleanest specimen: written, unit-tested, and named in three XML doc comments as the startup enforcement for two bounds — one of them the size ceiling above which structural secret redaction stops being trustworthy — and bound into no options pipeline, so **not one of its rules had ever executed in any host**. Its own doc comment said "auto-discovered via `AddValidatorsFromAssembly` — no manual registration required", which is true of the DI registration and irrelevant to whether anything resolves it: nothing validates a config POCO unless an `AddOptions` chain asks it to. **Every available signal said the control was fine, because each measured something other than "does this run in a host"**: its own tests passed (they construct it directly), `ValidateOnBuildSweepTests` passed (an unbound validator breaks no service graph), and the interface-consumer scan in `SecurityControlHasACallerTests` passed (a validator implements no guarded contract). The prior five instances are recorded in that file's remarks — `ToolPermissionFilter`, `GoverningToolContextProvider`, both recall providers, and `IAgentIdentityValidator`, whose fail-closed RBAC had fifteen passing tests and no production caller until #311. **The rule: after writing any control, ask which single line, if deleted, restores the unguarded behaviour — then confirm a test fails when it is gone.** If none does, the control is untested however many tests surround it. Prefer a guard written over the whole *class* of control rather than the one that broke: `SecurityControlHasACallerTests.EveryConfigValidator_IsBoundIntoTheOptionsPipeline` was added for this instance and immediately reported five more — which turned out to be a **false alarm** (#514): the five planner step-config validators run via a consumer resolving `IValidator<T>` (`PlanValidator.ValidateStepConfigurations`), a second invocation mechanism the guard did not know about. They are now an explicit five-entry exemption list, kept honest by a staleness check anchored on the dispatch-arm shape in `PlanValidator` (matches exactly the five arms; mutation-tested by deleting the one arm whose type is mentioned elsewhere in the file). A first version of that check matched the bare type name and was blind for one of five; it was withdrawn on the claim that it "could not fail" — which was false, and removing it dropped real detection for the other four. Remaining limits are tracked in #528; the filename blind spot is live (#529, four Drift validators run nowhere). That guard produced five false alarms in total before reaching its current shape, two of them in the *dangerous* direction (silently exempting a real unbound validator) by trying to infer the exemption from a namespace rule and then a co-occurrence scan. The lesson: when the mechanism cannot be detected, state it and check the statement — do not approximate it with something that correlates. Two earlier cautions from writing that guard: match on the *shape* that needs wiring (`AbstractValidator<>`), not on a filename convention — matching `*ConfigValidator.cs` swept in two `IHostedService` validators that self-register and are live, and a guard that cries wolf is the fastest way to get a guard ignored; and scan **every** `DependencyInjection*.cs`, not just the composition root, or anything wired in a subsystem partial reads as unbound.
- **Narrowing a structural false-positive fix by adding a require-something condition — check both directions, not just the one the issue named.** `ToolResultText.TryGetContentArray` (#488) matched any JSON object with a top-level `content` array, so a keyed-DI tool's own domain schema could coincidentally be misidentified as an MCP result and get silently reparsed. Three rounds, each caught by independent security review, none by the two prior local passes: **round 1** required at least one of `isError`/`structuredContent`/`_meta` to be present — plausible, since those are the properties `McpFailureNormalizingAIFunction`'s own `isError` check already relied on, but wrong: `McpJsonUtilities.DefaultOptions` sets `JsonIgnoreCondition.WhenWritingNull`, and a spec-conformant success result omits all three whenever `McpClientTool.InvokeCoreAsync`'s `AIContent`-conversion fallback fires for *any* reason, not only structured content or metadata — confirmed against the real, pinned SDK, a `resource_link` block always triggers it with zero markers on the wire. That missed a real, adversary-triggerable MCP shape entirely, silently skipping sanitize/redact/bound for it — worse than the false positive #488 was filed against, and reachable by any MCP server appending one `resource_link` block to a response. **Round 2** required *every* element of the content array to look like a real block (an object with a string `type` discriminator, the protocol's actual structural invariant) — closed round 1's gap, but `TransformSerializedContentBlocks`/`ExtractContentArrayText` already tolerated a non-conforming element by skipping it one block at a time; requiring the whole array to be uniform meant one deliberately malformed element (trivial for the same hostile server to include) rejected the entire array, silently withholding sanitize/redact/bound from every *other* block in it too — including a genuine injection-bearing `text` block sitting right next to the malformed one. **Round 3**, the fix that shipped: require *at least one* qualifying block, not every element — strictly tighter than the original any-array check (a first-party array of bare strings, numbers, or all-typeless objects still never matches) without either prior failure mode. **The pattern across all three:** narrowing a check that's too permissive in one direction is easy to verify by testing the false-positive case it was filed against; verifying it didn't become too strict in the *other* direction — silently rejecting real data — needs its own explicit test, ideally built by tracing the actual wire format of a real producer (here, a throwaway console app against the pinned `ModelContextProtocol.Core` package settled two of the three rounds empirically, not by reasoning from the SDK's public docs or the issue's own stated premise). A test proving the false positive is fixed says nothing about whether a false negative was introduced fixing it.
- **Adding per-caller credentials to a shared MCP session cache without widening its cache key.** `McpConnectionManager` caches every host-configured and bundle-remote MCP client (`_clients`, `_entraClients`, `_bundleEgressClients`) keyed by bare server name — every caller/conversation/agent in the process shares one session per server. That is safe today only because no credential attached to a cached session is ever per-caller: static `McpServerAuthConfig` headers and Entra tokens are both scoped to the harness's own workload identity, never a request's caller (#748 audit). It is safe *by accident of what we don't do yet*, not by a check that would catch the alternative — the day someone adds on-behalf-of or delegated-user MCP auth, reusing these caches as-is silently hands caller B whatever session caller A already opened, the exact class of bug Microsoft's Agent Framework fixed upstream for its own provider-backed MCP sessions (PR #8425, "Scope provider-backed MCP sessions per invocation"). If per-caller MCP credentials are ever introduced, the cache key must include caller identity, not just server name — `McpConnectionManagerTransportTests.SharedCaches_AreKeyedByServerNameOnly` pins the current key *type* (a reflection check, since named tuple element names like `_runScopedClients`' `(ServerName, RunId)` are compiler metadata invisible to `Type` equality — this can only detect a type change, e.g. `string` widening to a tuple) so that specific shape of change can't happen as an accidental side effect of an unrelated refactor. It does **not** catch a caller identity folded into an otherwise-unchanged `string` key, or caller scoping added via an ambient accessor mirroring this same file's own `BundleRunIdAccessor` pattern (an `AsyncLocal` read inside `RequiresRunScope`) — both are real, undetected ways this invariant could still be silently broken, and need a human reading this entry, not a test catching it.
- **Hand-copying the child-governance-scope arming sequence at a new surface.** Opening a child DI scope, calling `IAgentExecutionContext.Initialize`, re-stamping workload identity and resetting the admission pipeline was written out separately in `DirectToolInvoker`, `SubPlanStepExecutor` and the capability supervisor's delegation, and the copies drifted: sub-plans dropped the parent's workload identity and accepted an empty conversation id, and delegation authorised nested calls against the wrong agent until #757. They now all go through `GovernanceArmer` with a `GovernanceArmingPolicy` preset (`DirectInvocation`, `SubPlan`, `Delegation`) that names each choice (conversation id, call-once scope, turn number); Magentic participants (#769) are the fourth surface, through `ParticipantGovernance`/`ParticipantGovernanceTurn` with the `Delegation` preset; a further surface picks or adds a preset instead of writing another copy. A surface that opens a **child scope** (delegation, participants) uses `GovernanceArmer.OpenChild`, which returns a `GovernedChildScope` that owns the scope and, on dispose, folds the child's trace into the turn's before releasing it (#805) — a hand-written fold is how a child's denials vanish from the turn's `Governance` trace (#771), and `GovernanceTraceIsFoldedByTheChildScopeHandleOnlyTests` forbids a second caller of `IGovernanceTraceRecorder.Absorb`. `Arm`/`ArmWithAdmission` alone do not own the scope or publish any ambient accessor, for the sub-plan and direct-invocation surfaces that manage their own. Surfaces that own their request scope (the MediatR behaviours, turn handlers, `PlanRunExecutor`) are the only ones outside it. Pinned by `GovernanceArmerTests`; the four surfaces keep their own characterization tests (`ParticipantGovernanceTests` and `MagenticAgentTurnRunnerTests` for the Magentic one), because the preset a surface picks is what the helper cannot test for it.
- **Publishing ambient state once around a streamed run.** Ambient (`AsyncLocal`) state set inside an async iterator does not survive a `yield`: the consumer resumes the iterator under its own context. A pipeline, envelope or attribution published once at the top of a stream governs only its first step, and every later tool call runs under whatever is ambient outside it. Publish it around every step (`ParticipantGovernanceTurn` sets and releases the admission pipeline around each `MoveNextAsync`; its streaming test fails if it is published once). Attribution is the one value you cannot re-publish by hand — `AttributionIsPublishedByTheExecutionContextOnlyTests` forbids a second `BeginTurn` outside `AgentExecutionContext`, because a nested publication releases out of order (#737). Re-assert it through `IAgentExecutionContext.ReassertAttribution()` instead, acquired and released inside the step like the pipeline (`ParticipantGovernanceTurn.BeginStep`); measured against the real SDK in `StreamedParticipantAttributionTests` (#803). Arming a child context inside the iterator also publishes a turn-long attribution scope; give it back at once with `IAgentExecutionContext.ReleaseTurnAttribution()`, because in a shared baggage holder a scope held across the yield leaves the participant's identity on the supervisor's spans for the rest of the request. A nested run must also publish its own agent id with its pipeline (`ToolAdmissionAccessor.Begin(pipeline, agentId)`, read back as `CurrentAgentId`), or the self-delegation check and authorization read the enclosing request scope's agent (#809).
- **Package skew: one assembly compiled against an older sibling than the one that loads.** It surfaces as a `MissingMethodException` on first call, possibly in production, and neither the compiler nor `ValidateOnBuildSweepTests` can see it. It landed repeatedly: the Agent Framework 1.24 upgrade (Azure.AI.OpenAI 2.9.0-beta.1 against OpenAI 2.14, since removed by moving the AzureOpenAI client type onto the OpenAI SDK's `/openai/v1/` route; FoundryHost on ModelContextProtocol 2.x under 1.4.1-compiled code), `Microsoft.EntityFrameworkCore.InMemory` 8 under EF Core 10, and `Microsoft.EntityFrameworkCore.Design` 8.0.11 shipped beside it, both pulled in by `AspNetCore.HealthChecks.UI`. Each was found only by a hand-written smoke test or a crash. `PackageSkewTests` in every host test project now asks the whole question: `PackageSkewGuard` loads the host's own build output the way the host binds it (`HostClosure`, framework wins over a lower app-local copy) and resolves every cross-assembly member reference (`MemberReferenceResolver`). A new break fails with no one having thought of it; fix it by pinning the transitive package in `Presentation.Common.csproj` (the CPM version), or accept it in `KnownHostSkews` **with a reason** — an entry that stops matching fails too, so a fixed skew has to leave the list. It sees missing members and types that will not load, not behaviour changes, and a reference on a generic instantiation is matched by name and parameter count rather than full signature; keep the runtime surface tests (`AzureOpenAiV1ClientTests` for Azure OpenAI, `FoundryDirectResponsesClientTests` and `AzureOpenAiSurfaceCompatibilityTests` for the two Foundry paths) for wire shape. It also cannot tell a call to a method that exists but is itself broken inside its package from a healthy one; when one has to be tolerated, keep it off production paths with a source-scan test, as was done for `AzureOpenAIClient.GetResponsesClient()` until #799 removed Azure.AI.OpenAI altogether. An allowlist entry for a package that is not shipped sets `Kind: MissingAssembly` so it stops excusing the references the day the package ships. A host with its own extra skew adds its own list, not an entry in `Shared`. A `Microsoft.Agents.AI` bump also runs `MafHostingPackagesCanaryTests` (project `Infrastructure.AI.MafHostingCanary.Tests`), which fails if the preview A2A or AG-UI hosting surface changed; when it does, re-read `documentation/architecture/a2a-message-contract.md` and `ag-ui-hosting-evaluation.md` before accepting the bump.

<!-- gitnexus:start -->

# GitNexus — Code Intelligence

This project is indexed by GitNexus as **microsoft-agentic-harness**. Use the GitNexus MCP tools to understand code, assess impact, and navigate safely.

> Index stale? Run `node .gitnexus/run.cjs analyze` from the project root — it auto-selects an available runner. No `.gitnexus/run.cjs` yet? `npx gitnexus analyze` (npm 11 crash → `npm i -g gitnexus`; #1939).

## Always Do

- **MUST run impact analysis before editing any symbol.** Before modifying a function, class, or method, run `impact({target: "symbolName", direction: "upstream"})` and report the blast radius (direct callers, affected processes, risk level) to the user.
- **MUST run `detect_changes()` before committing** to verify your changes only affect expected symbols and execution flows. For regression review, compare against the default branch: `detect_changes({scope: "compare", base_ref: "main"})`.
- **MUST warn the user** if impact analysis returns HIGH or CRITICAL risk before proceeding with edits.
- When exploring unfamiliar code, use `query({search_query: "concept"})` to find execution flows instead of grepping. It returns process-grouped results ranked by relevance.
- When you need full context on a specific symbol — callers, callees, which execution flows it participates in — use `context({name: "symbolName"})`.
- For security review, `explain({target: "fileOrSymbol"})` lists taint findings (source→sink flows; needs `analyze --pdg`).

## Never Do

- NEVER edit a function, class, or method without first running `impact` on it.
- NEVER ignore HIGH or CRITICAL risk warnings from impact analysis.
- NEVER rename symbols with find-and-replace — use `rename` which understands the call graph.
- NEVER commit changes without running `detect_changes()` to check affected scope.

## Resources

| Resource | Use for |
|----------|---------|
| `gitnexus://repo/microsoft-agentic-harness/context` | Codebase overview, check index freshness |
| `gitnexus://repo/microsoft-agentic-harness/clusters` | All functional areas |
| `gitnexus://repo/microsoft-agentic-harness/processes` | All execution flows |
| `gitnexus://repo/microsoft-agentic-harness/process/{name}` | Step-by-step execution trace |

## CLI

| Task | Read this skill file |
|------|---------------------|
| Understand architecture / "How does X work?" | `.claude/skills/gitnexus/gitnexus-exploring/SKILL.md` |
| Blast radius / "What breaks if I change X?" | `.claude/skills/gitnexus/gitnexus-impact-analysis/SKILL.md` |
| Trace bugs / "Why is X failing?" | `.claude/skills/gitnexus/gitnexus-debugging/SKILL.md` |
| Rename / extract / split / refactor | `.claude/skills/gitnexus/gitnexus-refactoring/SKILL.md` |
| Tools, resources, schema reference | `.claude/skills/gitnexus/gitnexus-guide/SKILL.md` |
| Index, status, clean, wiki CLI commands | `.claude/skills/gitnexus/gitnexus-cli/SKILL.md` |

<!-- gitnexus:end -->
