refactor: lean on the type system — required properties, primary constructors, no permissive defaults on security-relevant fields

[Aaronontheweb]

Background

PR #993 fixed a one-line omission in SlackThreadHistoryFetcher.ConvertMessageAsync that produced a silent Public-audience fallback for a Personal-audience DM — shell_execute was denied as tool_not_allowed_for_audience_profile even when the operator had Slack.ChannelAudiences[\"dm\"] = \"personal\" configured.

The bug was not a logic error. It was a type-shape error: ChannelInput.Audience is declared TrustAudience? Audience { get; init; } (nullable, optional), and MessageSourceFactory.Create invents a default via input.Audience ?? options.DefaultAudience. The compiler had no way to tell a forgetful caller from a deliberate one — and the fallback path silently downgraded trust.

The constitution already names both halves of this anti-pattern:

No silent fallbacks. When something fails or is misconfigured, fail loudly — do not silently degrade to a default. ... on security-relevant paths they can silently escalate privileges.

Value objects exist to prevent accidental misuse. ... Use .Value for explicit access ... If a value object can silently become a string, it provides no more safety than a raw string.

But the codebase has plenty of records and DTOs where security-relevant fields are still nullable-with-init-fallback rather than required (or constructor-injected). The audience field was the one that bit us first; it is unlikely to be the only one.

Proposal

Treat this as an umbrella effort to beef up the type system as a primary correctness gate, prioritizing security-relevant paths. Concrete moves:

1. Required properties on records carrying trust context

Audit all records that flow trust/identity through actor boundaries and make fields that have no legitimate "unset" state required. Starting set:

• ChannelInput — Audience, Principal, Provenance (plus probably Boundary)
• MessageSource — same fields (already mostly populated, but the type doesn't enforce it)
• SourceProvenance — TransportAuthenticity, PayloadTaint, SourceKind (no "strict default" callers actually want)
• ToolExecutionContext.Audience — currently a wire-string; should be the parsed TrustAudience so the parse failure is at construction, not at gate-check

For each: remove the ?? options.DefaultX fallbacks in MessageSourceFactory and friends; delete the corresponding DefaultX properties from SessionPipelineOptions once unused.

2. Primary constructors over property-init blocks for these records

Property-init records with sentinel defaults (= TrustAudience.Public, = SourceProvenance.StrictDefault()) make missing-field bugs invisible. Primary constructors with required positional parameters give the compiler real teeth — every callsite has to spell the value out.

This is more of a stylistic shift than a behavior change, but it reduces the chance that someone adds a new field with a permissive default "just to keep the diff small."

3. Audit pass: nullable-with-fallback on security-relevant fields

Sweep for the pattern someNullableProp ?? defaultValue where someNullableProp carries authorization, audience, principal, boundary, provenance, or transport-authenticity data. Each occurrence is a candidate for either:

• Making the source field required (preferred), or
• Failing loud at the consumer (throw instead of ??), or
• A documented justification that the fallback is genuinely safe.

The constitution rule for fallbacks already implies this — the audit just operationalizes it.

4. Out of scope (for now)

Same treatment for non-security-relevant nullable fields (e.g. ExecutableText, AdoptedContextProjection). Those have legitimate "absent" states and don't need this treatment.

Generator/serialization records (SerializableChatMessage, proto-defined types) — proto3 doesn't have a notion of required, so leave those to their own framework rules.

Why now

The release blocker that motivated PR #993 was discovered in production via an agent operator (me, in a DM). The exact same shape could be hiding in Principal or Provenance and we wouldn't notice until something similar happens. Better to spend a focused day strengthening the boundary types than to keep patching one-line omissions reactively.

Effort estimate

• Pass 1 (audit + plan): ~half day. Inventory of every nullable-with-fallback on trust-bearing types, with a row-by-row recommendation.
• Pass 2 (mechanical refactor): ~1-2 days. Required-property conversions are mostly compiler-driven; primary-constructor conversions touch every callsite.
• Pass 3 (delete dead defaults): cleanup of SessionPipelineOptions.DefaultAudience and siblings after the conversions land.

Worth splitting Pass 2 by record so each PR is reviewable in isolation.

[Aaronontheweb]

Partially addressed in #1001