Skip to content

Fix analyzer false positives, proxy generation defects, identity deserialization and observable removal - #2654

Merged
einari merged 34 commits into
mainfrom
fix/analyzer-generator-and-runtime-fixes
Sep 15, 2026
Merged

einari merged 34 commits into
mainfrom
fix/analyzer-generator-and-runtime-fixes

Conversation

@einari

@einari einari commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Summary

A batch of reported issues. Most were silent failures — things that went wrong without telling anyone. Two change runtime behavior rather than just a diagnostic and are worth reading before upgrading: collection query arguments now actually bind, and a single-instance observable query now notifies when the observed document is gone.

Added

Changed

Fixed

einari and others added 30 commits September 6, 2026 09:15
Property-level [Unique] had six specs through the command pipeline but
the event-type form (UniqueEventTypeConstraintsProvider) had none. Add
a PartnerOnboardingCompleted event with a class-level [Unique] and a
CompletePartnerOnboarding command, then cover: a second occurrence for
the same event source is rejected, the same event type for a different
event source is allowed, and the rejection's CommandResult carries the
violated constraint name in ReasonDetail.
The analyzer previously flagged any explicit id argument, including
one that intentionally differs from the type name to rename an event
record while stored events keep resolving under the old identifier.
Bind the id argument through the resolved constructor's parameter
list (surviving a reordered parameter list or an explicit generation
before the id), compare against the type's MetadataName (matching
Chronicle's ResolveId, which uses Type.Name) using an ordinal
comparison, and only report when the id is empty or equal to the
type name. A non-constant id is left alone rather than guessed at.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CB4UhBJEsRPanCrd8TxFrj
Delete and_id_is_specified, which asserted the exact behavior #2633
reported as wrong (flagging any explicit id, including one used to
rename an event record). Add coverage for the fixed analyzer: id
equal to or differing from the type name, positional binding proven
with a trailing generation argument, the named id: form, nameof,
a constant field, casing-only differences (ordinal comparison), and
an explicit empty string.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CB4UhBJEsRPanCrd8TxFrj
IIdentity<TDetails>.details was raw parsed JSON at runtime despite its
typed signature. Route both IdentityProvider deserialization call
sites (cookie and /.cratis/me) through a new deserializeIdentityDetails
helper that guards every unsafe path: no type, non-object payload, an
already-deserialized instance (re-deserializing is destructive, not
merely wasteful), and a type with no @field-decorated members (warns
and passes the raw payload through instead of silently blanking it).

Also repair two vacuous specs that declared an undecorated TestDetails
class and asserted only the field initializer default (Guid.empty),
never the value actually sent by the server - they passed regardless
of whether deserialization worked at all.
ArcProps had no way to configure type-safe identity details
deserialization even though IdentityProviderProps.detailsType already
supported it - <Arc> mounted IdentityProvider without ever passing it
through, so the only way to reach it was bypassing <Arc> and wiring
IdentityProvider directly.
useIdentity(type) used the constructor only to discriminate which
argument was the default, then discarded it - the "type-safe
deserialization" its own JSDoc promised never happened. It now:

- Recognizes when the provider (<Arc>/IdentityProvider) already
  deserialized the payload with this exact type, via the
  detailsConstructor the provider records, and hands back the same
  instance instead of deserializing it again - re-deserializing an
  already-deserialized value is destructive (it can throw on nested
  temporal values and double-wraps concepts), not merely wasteful.
- Otherwise deserializes through the new deserializeIdentityDetails
  helper, memoized in a module-level WeakMap keyed on the raw payload
  object then the target type, so repeated reads and multiple
  consumers share one instance without a React hook (this hook must
  stay callable outside a render tree - existing specs invoke it via
  sinon.stub(React, 'useContext')).
- Never deserializes the caller-supplied default - it is already
  typed, and deserializing it again would double-wrap it.

Also reorders the two overloads. TDetails is unconstrained, so a bare
class reference is assignable to the single-argument overload's
`defaultDetails?: TDetails` parameter, and TypeScript resolves
overloads in declaration order - with the type-taking overload listed
second, `useIdentity(SomeDetailsType)` silently resolved to the
defaultDetails overload, inferring TDetails as `typeof SomeDetailsType`
instead of the instance type. Listing the more specific overload first
is the standard fix and was exposed by the new specs added here.

Removes the render-phase mutation that wrote a consumer's default
straight into the shared provider identity object
(`identity.details = actualDefaultDetails`) - with two consumers
supplying different defaults, the last one to render won for both.
The fallback-to-default behavior is preserved through the resolution
chain instead of a shared-state write.
The docs taught the exact shape that silently loses data: an
undecorated details class. JsonSerializer.deserializeFromInstance()
only copies @field-declared members, so following the old examples
verbatim produced an empty instance with the payload discarded.

- react/identity.md: add @field decorators to the UserIdentityDetails
  example, lead with <Arc detailsType={...}> as the primary form,
  note useIdentity(type) is only needed when the provider wasn't
  given one (supplying both is safe), and that the default in the
  second parameter is never deserialized.
- react/arc.md: add the detailsType configuration option.
- core/identity.md, react.mvvm/identity.md: clarify that
  getCurrent<T>() with only a type parameter does not deserialize -
  the runtime constructor argument is what does.
- auth-and-identity skill reference: keep the same fixes in sync.
Chronicle dispatches to a handler by its first parameter's event type
using both public and non-public methods, and only the immediate
enclosing method mattered to the previous per-invocation analysis.
That produced false positives for a private helper shared by several
[OnceOnly] handlers (#2615) and for a live handler that has a
[Replay] sibling for the same event (#2632), while also being unable
to see a call reaching Execute through a chain of private helpers.

Replace the single-invocation rule with a per-reactor symbol-start
analysis that records every ICommandPipeline.Execute call and every
intra-type call edge, then at symbol end walks from each handler
Chronicle would actually dispatch to (public, or the sole non-public
candidate for its event type) through that call graph to the
executions it reaches. A handler is excused when it carries
[OnceOnly] or [Replay], or when a sibling handler for the same event
type carries [Replay] — declaring one is itself the statement that
replay was considered. Reporting is per execution call site so two
handlers sharing one helper produce a single diagnostic naming both.

Rename the descriptor field and message to stop asserting [OnceOnly]
as the only remedy, since a [Replay] handler is frequently the
correct one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CB4UhBJEsRPanCrd8TxFrj
Add [EventType] to the two existing specs' event record, since the
new handler-candidate test requires a real event type parameter to
even consider a method a handler; without it both specs would keep
passing for the wrong reason.

Cover the new decision surface: a private helper shared by several
[OnceOnly] handlers (#2615, no diagnostic), the walk-back from a
private helper to the public handler that reaches it, dispatch
precedence when a private method shares an event type with a public
one, the private-only-handler case dispatch actually selects, a
non-handler method with a non-event first parameter, reaching
Execute through a chain of two helpers, two handlers sharing one
helper (one diagnostic naming both), a call made from inside a
lambda, and a class-level [OnceOnly] reactor.

Add a new when_replay_is_handled_deliberately/ folder covering #2632
(a sibling [Replay] handler for the same event excuses the live
handler), a [Replay] handler for a different event (no excuse), and
the [Replay] handler itself being the one that calls Execute.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CB4UhBJEsRPanCrd8TxFrj
useIdentity's resolution only treated null/undefined details as
"nothing to give" - an anonymous or not-yet-resolved identity's
details sentinel is `{}`, which is neither, so a caller-supplied
default was silently ignored in exactly the situation it exists for:
a consumer reading `details.someField` got `undefined` instead of
their default.

Gate the very first resolution step on `isSet` instead: not set means
there are no real details regardless of type or the raw payload's
shape, so the default wins outright before deserialization is even
considered. `isSet` is used rather than checking whether `rawDetails`
looks empty, because `{}` is also what a real, deserialized details
type with no populated members looks like - guessing "absent" from
shape would misfire for that caller.

Everything else is unchanged: the default is still never deserialized,
there is still no render-phase mutation, and the WeakMap memo is
unaffected since the anonymous path now returns before ever reaching
it.
Both rules previously had no documentation page despite the index
claiming the ids are sequential without gaps, and both issues that
prompted the analyzer fixes (#2633, #2615, #2632) came from users
following the diagnostic message with no page to consult. Add pages
following the existing ARCCHR0003/ARCCHR0009 structure: ARCCHR0004
leads with the rename case the rule must not flag and states plainly
that removing a pinned id orphans stored events; ARCCHR0006 adds a
section on choosing between [OnceOnly] and [Replay], what the rule
cannot see (cross-type helpers, inherited handlers, an event type
reached only through a base type, a reactor that returns a command
instead of calling Execute), and how to suppress a genuine false
positive. Update the AnalyzerReleases.Unshipped.md notes to match
the renamed titles, and add both rules to the index and toc.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CB4UhBJEsRPanCrd8TxFrj
ExportTargetExistsOnDisk only probed for a same-named .ts/.tsx file, so
an export pointing at a hand-written sibling folder with its own
index.ts/index.tsx barrel (export * from './List';) was indistinguishable
from a stale export and got silently deleted on the next build. The
generator has never written a folder export, so this shape is always
hand-written and must never be pruned.

Also treat a non-relative (bare/package) specifier, e.g.
export * from '@cratis/components';, as always manual: it cannot be
verified on disk and is not something the generator could have written,
so it is now preserved unconditionally in both the staleness check and
the prune loop. Applied the same directory-awareness to
HasLiveNonGeneratedExports for symmetry.
Covers the folder-barrel regression directly: a generated file plus a
hand-written sibling folder with its own index.ts survives both a
no-op run and a run forced to rewrite by a new generated file, proves
the fix is idempotent across two consecutive runs, and confirms
pruning still removes the export once the folder or its barrel is
genuinely gone. Also covers a hand-added package re-export being
preserved, and the whole-output-path sweep discovering the folder
barrel without wrongly pruning the parent's export to it.
index.ts has always been hand-assembled in IndexFileManager.cs; nothing
ever invoked TemplateTypes.Index or constructed an IndexDescriptor
(confirmed by repo-wide search — only their own declarations referenced
them). Their presence was misleading enough that it was previously
mistaken for the code path actually responsible for index.ts content.
State the ownership model plainly: index.ts is never regenerated from
scratch, only merged into — an export is added only for a file just
generated in that directory, and removed only once its relative target
genuinely stops resolving on disk. Add examples for a hand-written
sibling folder barrel and a package re-export, and note the deliberate
exception that index.ts carries no @generated marker because marking
it would make it eligible for orphan deletion and would stop the
skip-generated-proxies ESLint processor from linting it.
ToQueryRequestParameterDescriptor and its controller-based twin,
ToRequestParameterDescriptor, stored the raw ParameterInfo.ParameterType
as OriginalType even when it was Nullable<TEnum>. IsKnownType() unwraps
Nullable<T> before consulting the primitive map, but an enum is never
in that map either way, so a nullable enum parameter's wrapper type —
not the enum itself — was what got collected into TypesInvolved.
Nullable<TEnum> is not itself an enum, so Generator.cs's enum split
missed it and ran it through ToTypeDescriptor as a plain class instead,
reflecting Nullable<T>'s own HasValue/Value properties into a bogus
type colliding with the enum's real name.

Unwrapping before anything else is derived from the type fixes this:
a nullable enum parameter is now tracked and emitted exactly like its
non-nullable counterpart. Optionality already comes from
ParameterInfo.IsOptional()/HasDefaultValue, not from the CLR type, so
this cannot change whether a parameter is treated as optional.
Pins the Nullable<T>-unwrap fix directly: a nullable enum parameter's
OriginalType now resolves to the enum, not the wrapper, and the
wrapper never reaches TypesInvolved. Also covers the plain-enum case
that already worked, an enum sitting next to an injected dependency to
confirm the two are still classified apart, and GetTargetType's
existing (previously unpinned) handling of a nullable enum.

Confirmed both new OriginalType/TypesInvolved facts fail without the
fix and pass with it; the full ProxyGenerator.Specs suite went from
1236 to 1250 passing with no new failures.
Add a nullable-enum example next to the existing plain-enum one in the
model-bound query-arguments guide, and state explicitly the rule that
previously existed only as an XML remark on IsQueryParameter: a
primitive, a concept, an enum, or a collection of primitives/concepts
is a caller-supplied argument; everything else is resolved from the
container. Note in the type-mapping reference that the enum/Nullable<T>
mapping holds in parameter position, not only property position.
Query.hbs and ObservableQuery.hbs both emitted
`readonly defaultValue: TModel = {} as any;` for a single-instance
query, regardless of the model type — the only `any` the generator
emits anywhere in checked-in output. Cast to the model type instead:
`{} as TModel`, using the triple-stache form the type annotation on
the same line already uses, since the double-stache form would
HTML-escape a dictionary model's angle brackets.

`{} as T` type-checks under strict for every shape the generator
emits (class, interface, primitive, array, enum, Record<,>, union,
generic); the only exception is `{} as void`, which cannot occur here
because a void/Task return leaves the model descriptor empty and the
template already emits the syntactically invalid
`readonly defaultValue:  = {} as any;` — both spellings are broken for
that pre-existing case, so this is not a regression.
Closes the zero-coverage gap on defaultValue: the existing query-proxy
spec always passes IsEnumerable: true and never reaches the
single-instance branch. Covers a read-model-typed default, the
observable-query template (which carries the identical line), a bare
primitive model (the shape already in TestApps/AspNetCore/
ObservableQueries.ts, which a naive fix could still get wrong), and a
dictionary model to pin that the triple-stache cast is not
HTML-escaped.

Confirmed all 8 as-any/defaultValue-typing facts fail against the
pre-fix templates and pass with the fix; the full suite went from
1250 to 1263 passing with no new failures.
The single-document observable's onNext projection only forwarded a
value when documents.FirstOrDefault() was non-null. After PR #2618
made HandleChange correctly remove a document from the observed set
on a hard delete, an update that moves it out of the filter, or an
empty initial query, that guard threw the resulting emission away
instead of forwarding it — leaving ObserveById waiting forever for a
document that will never (re)appear, and ObserveSingle silently
missing that a document left the result set.

Emit default unconditionally instead, matching this stack's existing
"no such document" convention (FindById already documents this) and
avoiding completing the observable, which would close the connection
out from under a subscriber rather than report absence.
The existing harness always blocked the SUT's change-stream loop on
Task.Delay(Timeout.Infinite, ...), so no spec could drive a delete,
update, or insert through HandleChange without live MongoDB. Back the
substituted cursor with a Channel instead, add PushChange plus
DeleteOf/InsertOf/UpdateOf change-document builders, a single-value
sibling of FirstEmission that can capture null, and an EmissionSequence
helper for specs that must observe more than one ordered emission
without racing the subject's replay buffer.

Add specs covering ObserveSingle and ObserveById: no match on the
initial query, a hard delete, an update that moves the document out
of the filter, an unrelated delete that must not re-emit, and a
delete-then-recreate proving the observer keeps working after
reporting "gone". Confirmed each of the null-emission specs fails
against the pre-fix ObserveSingle to prove they exercise the bug.
ObservableQueryDemultiplexer (the default hub-routed path) already
forwards a null emission unconditionally, and InterceptEmission is
explicitly null-tolerant. Direct-mode ClientObservable/ClientObservableSSE
disagreed: they returned early on a null value, dropping it instead
of sending a result with Data = null. Combined with the ObserveSingle
fix, an observed document going away now surfaces as a 200 with
data: null on every transport instead of a 408 timeout on direct mode.

Add a dedicated trace-level log message for the forwarded case so it
reads as "reporting absence", not "waiting for the next real value"
(which remains accurate for the per-item enumerable observables that
are deliberately left dropping null, since a null item there really is
nothing to send).
DbSetObserveExtensions had the character-for-character same
"if (result is not null)" guard as the MongoDB provider, plus a
Subject<T>-vs-BehaviorSubject<T> choice keyed off the same emptiness
check. An alternate implementation of the same concept must match the
primary one's semantics: apply the same decision — emit default
unconditionally, and always use BehaviorSubject<T> (seeded with
default when nothing matches) so a subscriber attaching after the
fact replays the current "no such entity" state instead of getting
nothing until the next change.

Add specs covering ObserveSingle and ObserveById: no match at start,
a delete, an update that moves the entity out of the filter, and an
unrelated write that must still report the real entity rather than
flipping to default. The SQLite-backed harness drives genuine
SaveChanges-triggered notifications, so these are true regression
specs, not mocked call assertions; confirmed the four bug-adjacent
ones fail against the pre-fix code.

Unlike the MongoDB provider, EF Core's change-tracking callback is
registered per table rather than per filter, so any write to the
table re-queries and re-emits regardless of relevance — a pre-existing,
unrelated characteristic the added specs account for rather than
change.
State that ObserveSingle()/ObserveById() emit null rather than
completing when the document is deleted, moves out of the filter, or
is never found, and show the result.hasData / result.isReady guard
for consuming it from React. Note that a [ReadModel] removed via
[RemovedWith<T>] hard-deletes its backing document, so this is the
same case as "the read model was removed" rather than a special one.
Note under Generated Artifacts that a single-instance query's
defaultValue is a typed empty placeholder (`{} as TModel`) and an
enumerable query's is `[]`.

Add a new page documenting @cratis/eslint-plugin-arc's
skip-generated-proxies processor, which detects the
`// @generated by Cratis` header and skips the file wholesale. It
shipped in 4dfeacd (2026-06-04) and was undocumented under
Documentation/**, which is why the reported issue's project had grown
a 13-entry hand-maintained ESLint path allow-list instead.
IServiceProviderIsService answers true for any IEnumerable<T> unconditionally,
so a query parameter like IEnumerable<int> ids was classified as an injected
dependency and silently resolved as an empty collection instead of the
caller-supplied values. Exclude a collection whose element type is a
primitive, a concept, or an enum from dependency resolution before asking
the container, mirroring the proxy generator's IsEnumerableOfPrimitiveOrConcept.

ConverterExtensions.ConvertTo also gains support for converting the
delimited string a collapsed repeated query key produces into that
collection, since classification alone does not bind the values.

Duplicated rather than shared with the generator's predicate: the generator
classifies MetadataLoadContext types with its own type-name-based concept
detection, while this runs against real loaded types via Cratis.Concepts -
genuinely different reflection contexts, not just different call sites.
Repairs the vacuous when_performing_query_with_enumerable_int_parameter_and_checking_url
spec, which only asserted the request URL and IsSuccess - both green whether
or not the server actually bound the values, which is exactly what let the
defect ship. Adds data assertions there and equivalent end-to-end scenarios
for a string collection and a collection of concepts (ProductCode), plus
focused Arc.Core.Specs coverage of the classification and value-binding
behavior directly, including a regression guard proving a genuinely
service-typed collection (IEnumerable<ISomeService>) still resolves from
the container.
Extends IsEnumerableOfPrimitiveOrConcept with an enum element check so a
collection-of-enum query parameter is classified as a caller-supplied
argument and gets a proxy, matching the runtime's own classification and
closing the second gap from #2571.
Pins IsEnumerableOfPrimitiveOrConcept's enum support with a unit spec, adds
EnumParameterReadModel.SearchByStatuses(IEnumerable<ReadModelStatus>), and
adds an end-to-end scenario asserting both the generated URL and the bound
data - the same pattern already used for primitive, string, and concept
collections.
States explicitly that a parameter is caller-supplied when it is a
primitive, a concept, an enum, or a collection of those - everything else
is resolved from the container - and adds a collection-of-enum example to
the Collection Arguments section alongside the existing primitive and
concept collection examples.
@einari einari added the minor label Sep 6, 2026
@einari

einari commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Verification and reviewer notes

Not release notes — recording what was verified and what was deliberately left alone.

Gates

  • dotnet build Arc.slnx -c Debug — 51 projects, 0 errors, 0 warnings
  • dotnet build Arc.slnx -c Release -p:CratisProxiesOutputPath= — 51 projects, 0 errors, 0 warnings
  • The Debug build produced no proxy drift (clean tree afterwards), so the regenerated TestApps proxies are stable and idempotent
  • dotnet test Arc.slnx -c Debug6759 passed, 28 failed

The 28 failures are pre-existing, not from this branch

  • 12 in Cratis.Arc.ProxyGenerator.for_Generator.when_generating_standard_decorator_metadata_from_the_command_line — confirmed identical against a pristine baseline via git stash on two separate occasions during this work. Worth their own issue.
  • 16 in Cratis.Arc.Core.Generators.Integration.Specs.for_AnalyzerPackageGraph — fixture-level, NETSDK1005: Assets file doesn't have a target for 'net8.0'. Directory.Build.props:31-36 multi-targets net8.0;net9.0;net10.0 in Release but net10.0 only in Debug, while the fixture runs dotnet pack -c Release --no-build --no-restore. A Debug-only restore cannot satisfy it. An artifact of running the whole solution locally in one Debug pass, not of any change here.
  • Decisive check: this branch changes 142 files and zero build-configuration files — no .props, .targets, .csproj, global.json, NuGet config or .slnx. It cannot produce a missing-TFM restore error.

Specs were proven non-vacuous, not just green

Several of these bugs existed because a spec passed without asserting the thing that mattered. Where a fix had a spec, the fix was stashed and the spec confirmed to fail first:

  • #2652: 13 of 16 new Arc.Core.Specs facts failed pre-fix; the 3 that passed are the IEnumerable<IService> regression guard, correctly unaffected. All 10 new data-assertion facts across 4 end-to-end scenarios failed while the URL-only facts stayed green — demonstrating directly that the original spec was vacuous.
  • #2642: 10 of 14 new MongoDB specs and 4 EF Core specs failed pre-fix; the 4 that stayed green are "should not emit again" regression guards.
  • #2613: all 13 new facts failed pre-fix.

The repaired spec is worth a look on its own — when_performing_query_with_enumerable_int_parameter_and_checking_url.cs asserted the URL the client produced and that the request succeeded, but never the data returned. Since SearchByIds projects from its argument, three items and zero items both passed.

Deliberately out of scope, filed instead

One history note

Commit 0d8ed372 carries the regenerated TestApps proxies alongside the MongoDB spec harness, under a message describing only the harness. Two agents were committing in one working tree and git commit takes the whole index. Content and causal ordering are correct — the template change in 5713f794 precedes its output — and per this repo's append-only history rule it was not rewritten over a commit message.

Comment on lines +58 to +61
catch
{
_typeScriptIsValid = false;
}
Comment on lines +57 to +60
catch
{
_typeScriptIsValid = false;
}
Comment on lines +58 to +61
catch
{
_typeScriptIsValid = false;
}
Comment on lines +58 to +61
catch
{
_typeScriptIsValid = false;
}
foreach (var ext in (string[])[".ts", ".tsx"])
{
var filePath = Path.Combine(directory, $"{match.Groups["path"].Value.TrimStart('.', '/')}{ext}");
var filePath = Path.Combine(directory, $"{fileName}{ext}");
/// <returns><see langword="true"/> if <paramref name="fileName"/> is a subdirectory of <paramref name="directory"/> with an <c>index.ts</c> or <c>index.tsx</c> file.</returns>
static bool DirectoryBarrelExists(string directory, string fileName)
{
var childDirectory = Path.Combine(directory, fileName);
{
var childDirectory = Path.Combine(directory, fileName);
return Directory.Exists(childDirectory) &&
(File.Exists(Path.Combine(childDirectory, "index.ts")) || File.Exists(Path.Combine(childDirectory, "index.tsx")));
{
var childDirectory = Path.Combine(directory, fileName);
return Directory.Exists(childDirectory) &&
(File.Exists(Path.Combine(childDirectory, "index.ts")) || File.Exists(Path.Combine(childDirectory, "index.tsx")));

void Establish()
{
_outputPath = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
FirstEmission disposes its own subscription, so taking the initial
emission through it dropped the subscriber count to zero. The subject
stops and disposes itself once the last subscriber leaves, so the
second subscribe raced that teardown instead of exercising replay.

It passed only because teardown used to be slow: the change-stream
cursor sat in an infinite delay, so cancellation took long enough for
the second subscribe to get in first. Making the cursor feedable
removed that accidental delay and CI failed with ObjectDisposedException.

Hold the first subscription open for the spec instead, which is what
"subscribing after the initial query" means - a late subscriber, not
one arriving after every subscriber left.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CB4UhBJEsRPanCrd8TxFrj
@einari

einari commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Correction to the verification note above

CI proved my characterization of the local test failures wrong in one respect, so correcting it here rather than leaving it to mislead a reviewer.

I described the 12 for_Generator failures as pre-existing. They are not — CI's proxy-specs (Cratis.Arc.ProxyGenerator.for_Generator) passes. They reproduce against a pristine local baseline, which is what led three separate checks to that conclusion, but the cause is environmental in the same family as the for_AnalyzerPackageGraph ones, not a real defect. No follow-up issue is needed for them.

package-graph also passes, confirming the 16 for_AnalyzerPackageGraph failures were the Debug-vs-Release restore artifact.

So all 28 local failures were environmental, and CI found exactly one real problem — a good reminder that a local full-solution dotnet test is not the gate CI is.

The one real failure, and why it matters

and_subscribing_after_the_initial_query — a pre-existing spec — failed with ObjectDisposedException. It was already timing-coupled:

FirstEmission disposes its own subscription, which drops LifetimeAwareSubject's subscriber count to zero. The subject stops and disposes itself once the last subscriber leaves (LifetimeAwareSubject.cs:84-98), and the spec then subscribed a second time. It only ever passed because teardown used to be slow — the change-stream cursor sat in Task.Delay(Timeout.Infinite), so cancellation propagated slowly enough for the second subscribe to win the race. Making the cursor feedable for the new delete specs removed that accidental delay and the latent race became deterministic.

This is exactly the failure mode .ai/rules/specs.csharp.md warns about: a spec that passes because the machine happened to be fast enough, then breaks on an unrelated change.

Fixed in f5e105c2 by holding the first subscription open, so the spec tests what its name says — a late subscriber seeing the replayed value, not one arriving after every subscriber left. Verified 6/6 clean runs in CI's exact configuration (--no-build -c Release --framework net10.0).

CI is now green: 60 checks passing, 0 failing.

…r-and-runtime-fixes

# Conflicts:
#	.ai/skills/auth-and-identity/references/frontend.md
#	Documentation/backend/chronicle/code-analysis/index.md
#	Documentation/backend/chronicle/code-analysis/toc.yml
#	Documentation/backend/mongodb/observing-collections.md
#	Documentation/backend/proxy-generation/file-index-tracking.md
#	Documentation/backend/proxy-generation/index.md
#	Documentation/backend/proxy-generation/queries.md
#	Documentation/backend/queries/model-bound/observable-queries.md
#	Documentation/backend/queries/model-bound/query-arguments.md
#	Documentation/frontend/core/identity.md
#	Documentation/frontend/react/arc.md
#	Documentation/frontend/react/identity.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant