refactor(domain): migrate AgentId to Vogen value object#513
Merged
Conversation
Adds central package versions for the Vogen value-object source generator (8.0.5) and NetArchTest.Rules (1.3.2). Wires Vogen into src/domain so the source generator is available for the AgentId migration. NetArchTest is consumed by the new architecture-fitness- function project. Refs: #500 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Removes two unused public types ahead of the AgentId Vogen migration: - BotNexus.Domain.Primitives.SessionStatus (smart-enum class) was an orphan duplicate of the Gateway SessionStatus enum used everywhere (aliased as GatewaySessionStatus). The class is missing the Expired value and is referenced by nothing in production. Removing it collapses the duplicate-types confusion called out in F-4. - BotNexus.Domain.Serialization.AgentIdJsonConverter — Vogen generates the System.Text.Json converter from [ValueObject(conversions: Conversions.SystemTextJson)], so the hand-rolled converter is no longer wired in. Its companion test file is deleted with it. Refs: #500 (Phase 1 — Vogen seed PR, finding F-4 and F-13) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Migrates BotNexus.Domain.Primitives.AgentId from a hand-rolled
readonly record struct to a Vogen-generated [ValueObject<string>]
with System.Text.Json conversion.
Why this change matters (F-13):
* Closes the silent-cast footgun. The previous AgentId had implicit
operator string and implicit operator AgentId(string), defeating the
whole point of a typed identifier. Vogen does not emit implicit
conversions, so call sites must use .Value (out) and .From(...) (in)
explicitly.
* Closes 'default(AgentId) is constructable' — the Vogen analyser
(VOG009) rejects default(AgentId) and new AgentId() at compile time.
* Replaces the auto-generated nonsense XML docs ('Represents struct.',
'Executes from.') with a single accurate validation contract comment.
* Replaces the hand-written AgentIdJsonConverter (deleted in the
previous commit) with the Vogen-generated converter.
* Unifies validation: AgentId.From(null/empty/whitespace) throws
ValueObjectValidationException consistently.
Call-site changes:
* Production: ~25 files updated to use AgentId.From(...) at boundaries
(deserialisation, REST/SignalR contracts, persistence) and .Value
when serialising back to string. Three GatewayHub normalizers stay
hand-rolled for the still-string-typed SessionId and ChannelKey
primitives; NormalizeAgentId is now a pass-through.
* Production dead-code removal: three defensive branches that handled
the old 'default(AgentId)' or 'AgentId with empty Value' cases are
removed (AgentDescriptorValidator, AgentsController.Update empty-
payload branch, GatewayHub.NormalizeAgentId try/catch). The route-
vs-payload mismatch check in AgentsController.Update is preserved.
* Tests: ~60 files updated with the same wrap/unwrap pattern. Three
test cases that constructed default(AgentId) (which is now blocked
by Vogen analyser) are migrated to AgentIdTests.From_RejectsNull /
From_RejectsEmptyOrWhitespace — the scenario is now structurally
prevented rather than runtime-checked.
* Test exception expectations updated where AgentId.From now throws
ValueObjectValidationException instead of ArgumentException; cross-
primitive tests now expect per-primitive exception types until
SessionId/ChannelKey are also Vogen-migrated (future PRs).
SessionId, ConversationId, ChannelKey, SenderId, ToolName,
ChannelAddress, ThreadId remain hand-rolled and will be migrated
(or deleted, in ThreadId's case) in subsequent PRs of Phase 1.
Refs: #500 (Phase 1 — Vogen seed PR, finding F-13)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… fitness functions Adds a new test project that enforces structural conventions at build time, rather than relying on documentation discipline alone. This is the antidote to the kind of drift catalogued in the domain-model review (AGENTS.md says one thing, code says another — F-8, F-13, recurrence of F-4). Initial rules (5): * Domain_HasNoProjectReferences — BotNexus.Domain references only framework assemblies and Vogen's source-generator runtime types. * AgentId_IsVogenValueObject — the AgentId type carries [ValueObjectAttribute<>]. * AgentId_HasNoImplicitStringConversion — defends against re-adding the implicit operator string footgun that motivated this migration. * DomainPrimitives_VogenValueObjectsAreStructs — Vogen value objects in Primitives/ must be value types (catches accidental class usage). * Domain_HasNoTwoPublicTypesWithTheSameSimpleName — defends against recurrence of the SessionStatus duplicate types (F-4). The project uses NetArchTest.Rules for one rule and direct reflection for the others (the reflection-based rules read more clearly than the fluent API would for attribute lookup and implicit-operator scans). Future Phase 1 PRs will add rules covering Contracts/Abstractions dependencies, no-string-Ids-on-public-contracts, and no-ThreadId (after Phase 6b deletes it). Per the plan §9.4. Refs: #500 (Phase 1 — Vogen seed PR, §9.4) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds the 'all new value objects use Vogen' convention to the root AGENTS.md and a worked AgentId example to src/domain/AGENTS.md. The convention: * New domain identifiers and scalar value objects are defined as Vogen [ValueObject<T>] in BotNexus.Domain.Primitives. * No implicit operator T to/from the backing primitive — .Value and .From(...) are explicit so the type boundary stays visible. * Validation lives in the Validate partial method; the JSON converter is generated via Conversions.SystemTextJson. * New hand-rolled value-object structs will be rejected by the architecture fitness functions in BotNexus.Architecture.Tests. Refs: #500 (Phase 1 — Vogen seed PR, §8.1) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced May 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 1 — Vogen seed PR (the first PR of the BotNexus domain-model cleanup).
Migrates
AgentIdfrom a hand-rolledreadonly record structto a Vogen[ValueObject<string>], establishes the Vogen + NetArchTest toolchain, and introduces the architecture-fitness-function project that will enforce the new conventions structurally from this point forward.Closes #500 (partial — establishes the Vogen seed and migrates the first of the three IDs scoped to that issue;
ConversationIdandSessionIdfollow in subsequent PRs so each diff stays reviewable).Why
The hand-rolled primitive structs in
BotNexus.Domain.Primitiveshad three documented problems (review §F-13):AgentIdandSessionIdhadimplicit operator stringandimplicit operator AgentId(string), defeating the entire point of strong typing —someConversationMethod(agentId)compiled silently.AgentId.Fromtrimmed and rejected whitespace;ChannelAddress.Fromdid neither;ThreadId.Fromchecked neither. Different IDs, different rules.AgentId.cs:Represents struct./Executes from.Vogen fixes all three structurally:
default(AgentId)andnew AgentId()at compile time..Valueand.From(...)are explicit.Validatepartial method); JSON / EF converters are generated.What's in this PR
build(deps): add Vogen and NetArchTest package versionsrefactor(domain): delete orphan SessionStatus and AgentIdJsonConverterrefactor(domain): migrate AgentId to Vogen value objectAgentId.cs+ ~85 call-site files + 3 dead-defensive-branch removals (see below).test(architecture): introduce BotNexus.Architecture.Tests with domain fitness functionsdocs(agents): document Vogen value-object conventionAGENTS.md+src/domain/AGENTS.md.Tests migrated (per AGENTS.md rule 7: tests are never net-deleted, they are migrated)
Three test cases that constructed
default(AgentId)became uncompilable under the Vogen analyser. They were migrated — not deleted — toAgentIdTests.From_RejectsNullandFrom_RejectsEmptyOrWhitespace. The scenarios they tested (defensive handling of empty AgentId on the wire) are now structurally prevented, so the matching dead code branches in production were removed in the same commit:AgentDescriptorValidator— empty-AgentId.ValuecheckAgentsController.Update— empty-payload-AgentId backfill branch (route-vs-payload mismatch check is preserved)GatewayHub.NormalizeAgentId— try/catch aroundAgentId.From(now a pass-through; theSessionIdandChannelKeynormalisers retain their try/catch because those primitives are still hand-rolled)Architecture fitness functions added (§9.4)
5 initial rules in
BotNexus.Architecture.Tests:Domain_HasNoProjectReferencesAgentId_IsVogenValueObjectAgentId_HasNoImplicitStringConversionDomainPrimitives_VogenValueObjectsAreStructsDomain_HasNoTwoPublicTypesWithTheSameSimpleNameMore rules are added as later Phase 1 PRs land (Contracts/Abstractions deps, no-string-Ids-on-public-contracts, no-ThreadId after Phase 6b).
What's NOT in this PR (deliberately)
ConversationId/SessionIdVogen migration — separate PR each, keeps the diffs reviewable.ChannelKey/ChannelAddress/SenderId/ToolNamemigration — later in Phase 1.ThreadId— deleted by Phase 6b, not migrated. The Vogen sweep skips it on purpose.Validation
dotnet build BotNexus.slnx --nologo --tl:off— 0 warnings, 0 errors.dotnet test BotNexus.slnx --nologo --tl:off --no-build— 3818 passed, 40 skipped (the 5 new architecture tests + the existing 3813).Plan references
PrimitiveValidationInconsistencyTeststo be added next PR alongside theConversationIdmigration)SessionStatus(closed by commit 2)AgentIdby commit 3)