Enforce tenancy at subject construction and default the type registry

[bruth]

No description provided.

[synadia-claude-reviewer]

Claude finished @bruth's task in 3m 21s —— View job

---

Review complete

• Gather context (diff, CLAUDE.md, changed files)
• Review typeregistry.go (new default registry)
• Review eventstore.go changes (subject construction, tenancy enforcement)
• Review manager.go / reactor.go changes
• Review test changes (eventstore_test.go, tenant_test.go, evolve_bench_test.go, helpers_test.go)
• Post summary review

Summary

This is a clean, well-scoped refactor: tenancy enforcement moves from per-operation convention into the subject-construction path (subjectPrefix/filtersToSubjects), and the nil-registry special case is replaced with a degenerate binaryTypes{} registry so there's no types == nil branching scattered across call sites. Confirmed no leftover nil-registry checks remain anywhere in the codebase.

Tenancy enforcement — traced every mutating path (Append, Decide, DecideAndEvolve, Evolve, Watch, CreateReactor/UpdateReactor/CreateOrUpdateReactor, DeleteReactor) and all funnel through requireTenant() either structurally (via subjectPrefix/filtersToSubjects) or explicitly where no subject is built (DeleteReactor). Decide/DecideAndEvolve also guard up front so an unscoped handle can't run decider side effects before failing at Append. Tenant filter tokens can't be used to escape scope since the tenant token is a fixed prefix ahead of any user-supplied filter pattern. Good structural fix — this closes a class of bugs where a future new operation could forget the tenant check.

Default registry (typeregistry.go) — binaryTypes preserves the historical no-registry wire format (raw []byte/BinaryMarshaler passthrough, codec.Binary, caller-owned type names), and it's pinned by TestNoRegistryWireCompat, which is exactly the right test to have here given this is a wire-compatibility concern for existing deployments. WithRegistry(nil) correctly no-ops rather than installing a registryTypes{r: nil} that would panic on first use.

Minor observations (not blocking):

• reactor.go's DeleteReactor only checks that some tenant scope is set, not that the reactor being deleted actually belongs to that tenant — since durable names are stream-global (per the existing GetReactor doc comment), a handle scoped to tenant "acme" can delete a same-named reactor durable created under tenant "beta". This is pre-existing/documented behavior, not introduced by this PR, but worth keeping in mind if tenant isolation guarantees get tightened later.
• No correctness issues found in Evolve's sequence/ack handling or Append's batch-publish sequence stamping — unchanged by this PR.

Test coverage is thorough (structural unit tests without a server plus integration tests with a real embedded NATS server for isolation, persistence-across-Get, and reactor round-tripping). No security or JetStream durability concerns identified.

[synadia-claude-reviewer]

Claude finished @bruth's task in 3m 22s —— View job

---

Review complete

• Gather context (diff, changed files)
• Review typeregistry.go (new default registry)
• Review eventstore.go changes (subject construction, tenancy enforcement)
• Review manager.go / reactor.go changes
• Review test changes
• Post final review summary

Summary

Second pass over this refactor — subject construction now enforces tenancy structurally (subjectPrefix/filtersToSubjects), and the nil-registry special case is replaced with a binaryTypes{} default registry. I traced the mutating paths (Append, Decide, DecideAndEvolve, Evolve, Watch, reactor create/update/delete) and confirmed every one funnels through requireTenant(), either structurally via subject construction or explicitly where no subject is built (DeleteReactor). No correctness issues in the tenancy enforcement itself, and TestNoRegistryWireCompat correctly pins the wire format so the registry refactor can't silently break existing deployments.

One real (but pre-existing, not introduced by this diff) finding, left as an inline comment on typeregistry.go:72-77: binaryTypes.resolveType (and registryTypes.resolveType) only requires declared to be non-empty — there's no character-class validation on the event Type token like validEntity performs on the entity. Since eventSubject builds the published subject as event.Entity + "." + event.Type, a caller-supplied Type containing ., *, >, or whitespace can inject extra subject tokens or wildcards, the same hazard validEntity exists to prevent on the entity side. This isn't new to this PR, but this PR does consolidate type-resolution into typeregistry.go as the single seam for it, making it a natural place to close the gap.

No JetStream durability/ack concerns and no public API breaks — EventStoreConfig.Tenancy and the new Err* sentinels are additive.

[synadia-claude-reviewer]

Minor/pre-existing, not introduced by this PR: binaryTypes.resolveType only checks that declared is non-empty — it doesn't validate the type token the way validEntity validates the entity. Since eventSubject (manager.go:107) builds the published subject as event.Entity + "." + event.Type, a caller-supplied Type containing ., *, >, or whitespace can inject extra subject tokens or wildcards into the published subject, the same class of problem validEntity was written to prevent on the entity side. Worth a follow-up to apply the same character-class check to the type token here (and in registryTypes.resolveType, since the registry's nameRegex permits dots in type names too), for symmetry with the entity-side guard.

Fix this →

[synadia-claude-reviewer]

Claude finished @bruth's task in 1m 34s —— View job

---

Review complete

• Gather context (diff, changed files, prior review comments)
• Review typeregistry.go — check if event-type validation (flagged in prior review) was addressed
• Review eventstore.go tenancy/subject construction changes
• Review manager.go / reactor.go changes
• Review test changes
• Post final review summary

Summary

This latest push (bdbd5b7 "Validate the event type as a single subject token") closes the gap flagged in the two prior automated reviews on this PR.

Type validation — fixed. wrapEvent in eventstore.go:385-392 now runs the resolved type through a new validType (eventstore.go:89-104) after s.types.resolveType, rejecting anything containing ., *, >, or whitespace with ErrEventTypeInvalid. Because this check happens in wrapEvent after resolution rather than inside resolveType itself, it applies uniformly to both binaryTypes and registryTypes — including the case where a *types.Registry permits dotted type names but the subject grammar can't represent them. TestTypeValidation and TestDottedRegistryTypeRejected (eventstore_test.go:942-979) pin exactly this, including the registry-allows-dots-but-subject-rejects-them conflict.

Reactor tenant scoping — DeleteReactor/UpdateReactor/CreateOrUpdateReactor all now call requireReactorScope, which looks up the existing durable and checks reactorInTenantScope before mutating (reactor.go:249-265, 358-376). A tenant-scoped handle can no longer delete or overwrite a same-named durable owned by another tenant — it reads as ErrReactorNotFound/ErrReactorExists instead. TestTenantReactorMutationScoped (tenant_test.go:367-405) covers this directly, including that the foreign durable is left untouched and the true owner retains full mutation rights.

Tenancy enforcement (unchanged from prior passes, re-verified) — every mutating path (Append, Decide, DecideAndEvolve, Evolve, Watch, reactor create/update/delete) funnels through requireTenant(), either structurally via subjectPrefix/filtersToSubjects or explicitly where no subject is built. The Expect pattern's strings.LastIndex(subject, ".") slicing (eventstore.go:717-718) is safe given validEntity/validType now guarantee the subject has exactly the expected token structure.

Default registry (typeregistry.go) — binaryTypes{} correctly preserves the historical no-registry wire format, pinned by TestNoRegistryWireCompat, and WithRegistry(nil) no-ops rather than installing a registry that would panic on first use.

No new correctness, security, or JetStream durability issues found in this update. Both previously-open findings are now resolved with direct test coverage.