Skip to content

Commit 22940c4

Browse files
committed
CSHARP-5943: Add per-area AGENTS.md files and reviewer sub-agents
Partition the driver into 13 functional areas. Each gets an AGENTS.md (with sibling CLAUDE.md @-pointer) at its directory root, plus a read-only reviewer sub-agent in .claude/agents/. The root AGENTS.md gains a functional-area index. docs/agents-architecture.md describes the layout for reviewers and future maintainers. Added cross-cutting reviewers and review skill Fix inaccuracies in per-area AGENTS.md files flagged by /review-areas Applied all documentation corrections identified by the multi-area branch review. No production source changes. | File | Change | |---|---| | src/MongoDB.Bson/AGENTS.md | Clarified conventions run first; attributes are applied on top as explicit overrides, not just tie-breakers | | src/MongoDB.Driver/Core/Operations/AGENTS.md | Fixed "sleep with backoff" — retry executors retry immediately against a different server, no back-off | | src/MongoDB.Driver/Authentication/AGENTS.md | Distinguished ECS task-role IMDS from AssumeRoleWithWebIdentity in AWS chain; noted PLAIN-without-TLS enforcement gap is fixable, not intentional | | src/MongoDB.Driver/Linq/AGENTS.md | Fixed scope glob to cover root-level files; rewrote operator-addition steps to separate expression-level vs pipeline-level paths | | src/MongoDB.Driver/GridFS/AGENTS.md | Corrected Delete order (.files first, chunks second, exception after both); clarified omitting revision is identical to -1 | | src/MongoDB.Driver/GeoJsonObjectModel/AGENTS.md | Qualified CCW winding as MongoDB 2dsphere-specific; noted GeoJson2DCoordinates is for 2d indexes only; added discriminator dispatch note; added round-trip test requirement | | src/MongoDB.Driver.Encryption/AGENTS.md | Corrected SigningRSAESPKCSCallback from RSA-OAEP to RSASSA-PKCS1-v1_5/SHA-256; added cross-reference distinguishing LIBMONGOCRYPT_PATH from CRYPT_SHARED_LIB_PATH | | src/MongoDB.Driver/Encryption/AGENTS.md | Added note that NoopBinaryDocumentFieldCryptor activates when no factory is registered in AutoEncryptionProviderRegistry | | src/MongoDB.Driver/Core/Logging/AGENTS.md | Fixed MaxDocumentLength -> MaxDocumentSize (actual property name) | | src/MongoDB.Driver/Core/Events/AGENTS.md | Updated CMAP event names to match actual type names (e.g. ConnectionPoolCheckingOutConnectionEvent) | Add optional head-ref argument to /review-areas The skill now accepts an optional second positional argument for the end of the diff range (default: HEAD). This allows reviewing an arbitrary commit range rather than always diffing to HEAD. Examples: /review-areas -> main...HEAD (unchanged default) /review-areas HEAD~3 -> HEAD~3...HEAD (last 3 commits) /review-areas abc123 def456 -> abc123...def456 (specific range) /review-areas --all HEAD~5 HEAD~2 -> all reviewers, restricted range Also standardize AGENTS.md frontmatter key to reviewer-agent throughout; src/MongoDB.Driver/AGENTS.md was using the plural reviewer-agents, which diverged from the convention in docs/agents-architecture.md. Fix more inaccuracies in per-area AGENTS.md files flagged by /review-areas Second pass on AGENTS.md accuracy after re-running /review-areas --all. The reviewers cross-checked each area doc against current source and flagged claims that no longer matched the code (or never did). Notable corrections by area: - BSON: JsonOutputMode enum members (Shell, CanonicalExtendedJson, RelaxedExtendedJson — no Strict); BSON corpus is under Specifications/bson-corpus/, not tests/MongoDB.Bson.Tests/Specs/; soften the LookupSerializer resolution-order claim. - Core (transport/SDAM): ClusterType has no DirectConnection member; ServerType uses ReplicaSetPrimary/ShardRouter/etc., not the spec short-names; SnappyCompressor uses Snappier, ZlibCompressor uses SharpCompress.Compressors.Deflate.ZlibStream; TcpStreamSettings exposes ReadTimeout/WriteTimeout/ConnectTimeout/SocketConfigurator, not SocketTimeout/KeepAlive; replace CertificateFilePath fiction with the real SslStreamSettings surface; drop the C#-doesn't-do-this fork-detection-via-ProcessId story; DnsMonitor uses 60s with min-TTL override (no LB-specific 10s); replace 4.6+/4.7+ hello version claims with helloOk negotiation. - Operations: AsyncCursor holds an IChannelSource (not a Handle); ChangeStreamOperation is a read op that returns a cursor, not a cursor itself; AggregateToCollectionOperation is non-retryable; describe the binding decorator/leaf split correctly and call out deprioritizedServers / IMayUseSecondaryCriteria; w:0 throws at StartTransaction time, not per-write; document the adaptive backoff applied to SystemOverloadedError retries; note retryable reads pass transactionNumber: null; reformat the session-layer table; scope the "no SemVer types here" guidance to Operations/Bindings. - Authentication: drop the nonexistent CreateMongoDBCR fallback advice; describe the AWS chain as delegated to AWS SDK FallbackCredentialsFactory; ISaslStep returns a (BytesToSendToServer, NextStep) tuple, not a named SaslStepResult type; add OidcSaslStep.cs/OidcCredentials.cs and Gssapi/SecurityContextFactory.cs to the file lists. - Driver root (router): soften "form the public API surface" and cross-reference the public surface in Core/, GridFS/, Search/, etc.; drop angle brackets from filenames and call out that MongoDatabase is internal sealed; clarify EstimatedDocumentCount is on the collection, not on IFindFluent; describe the CreateVectorSearchIndexModelBase hierarchy; note Field/ArrayFilter as definition-only; add string (JSON) to the implicit-conversion list; differentiate change-stream pipeline input types per watch level and clarify $out/$merge is server-rejected; move AggregateExpressionDefinition out of "stage options"; mention MaxAwaitTime; tighten the test filter; drop the client-bulk-write/ reference (no such directory). - LINQ: ClientSideProjectionDeserializer is at the public root, not Linq/Serializers/; the in-LINQ helper is ClientSideProjectionHelper in Misc/; replace invented "strict-mode/allow-server-side-eval" with the real CompatibilityLevel/EnableClientSideProjections flags; rename VariableBindings to SymbolTable; narrow Finalizers to First/Single variants. - GridFS: replace nonexistent DownloadStreamForwardOnly API with GridFSDownloadOptions { Seekable = true }; move Revision to GridFSDownloadByNameOptions; rephrase the chunk-size limit; drop the reference to a nonexistent Specifications/gridfs/ runner. - Search: replace invented QueryVector.Embedded/.BinaryVector factory methods with the real implicit-conversion surface; distinguish VectorSearchOptions (top-level stage) from VectorSearchOperatorOptions (compound-embedded); split SearchMetaResult (facets) from SearchSpanDefinition (span queries). - Encryption: drop ECC mention (HelperCollectionForEncryption only has Esc and Ecos); fix path to AutoEncryptionLibMongoController.cs; reword duplicate-registration semantics; add async pairs and IDisposable to the controller contract; qualify the "default 60s" KeyExpiration claim as libmongocrypt-side. - Events: IEvent is internal; describe the cached-handler dispatch rather than "cluster optimizes dispatch"; remove the nonexistent ConnectionReadyEvent (Ready is a pool event); add the *ing CMAP variants alongside *ed. - Logging: LoggingSettings ctor parameter is maxDocumentSize, not maxDocumentLength. - GeoJSON: clarify the two-tier GeoJsonObject<T>/GeoJsonGeometry<T> hierarchy (Feature/FeatureCollection extend GeoJsonObject<T> directly); soften the 2D-coordinates "do not use with 2dsphere" rule to a recommendation; mention that there are two parallel discriminator dispatchers (GeoJsonObjectSerializer and GeoJsonGeometrySerializer) that both need updating when adding a geometry type. - Spec conformance: drop the nonexistent JsonDrivenTestCaseFactory; UnifiedTestOperations/ is a sibling of Specifications/, not a subdirectory; replace the spec-area list with the actually-present directories (no gridfs/ or command-logging-and-monitoring/). - TestHelpers: drop the nonexistent MockCluster; clarify that RequireEnvironment and the timeout-enforcing framework live in the sibling MongoDB.TestHelpers project, not here; rename TimeoutEnforcingTestRunner reference to the actual registered class TimeoutEnforcingXunitTestFramework. Fix more inaccuracies in per-area AGENTS.md files flagged by /review-areas Apply per-area and cross-cutting reviewer findings from `/review-areas --all`: BSON: drop GuidRepresentationMode V2/V3 framing (mode was removed); clarify that GuidSerializer requires an explicit GuidRepresentation and Unspecified throws; correct registry-locking note (ConcurrentDictionary plus BsonSerializer.ConfigLock); reframe conventions/attributes as ordering within ConventionRunner; add type-confusion warning for polymorphic deserialization of untrusted BSON. Core (transport): keep ReplicaSetPassive in the ServerType list (still public, [Obsolete]); correct heartbeat default (interval 10s, timeout Timeout.InfiniteTimeSpan); NotPrimaryError marks server Unknown; streaming heartbeat is the default for helloOk-supporting servers; helloOk landed in 5.0; MaintenanceHelper.EnsureMinSize proactively warms the pool; LB enum names are ClusterType/ServerType.LoadBalanced; moreToCome describes server exhaust responses, not client batched getMore; cross-ref DnsClientWrapper. Core/Operations: list IExecutableInRetryable*Context interfaces in IRetryableOperation.cs; soften "all four code paths" to sync/async with optional retryable-context overload; correct retryable-error label to RetryableWriteError; clarify AggregateToCollectionOperation implements IRetryableWriteOperation but reports IsOperationRetryable => false; note public types in Core/Bindings/ are SemVer-sensitive. Authentication: remove non-existent MongoCredential.CreateAwsIam factory; note AWS does NOT support speculative auth; correct OIDC environments to test/azure/gcp/k8s and clarify ENVIRONMENT vs OIDC_ENV (test-only); drop the "human" flow; reauthentication (error 391) GA'd in 7.0; tighten SaslMechanisms property reference. Update AWS AGENTS.md cross-ref to MongoClientSettings.Extensions.SaslMechanisms. Driver root: implementations (MongoDatabase, MongoCollectionBase, MongoCollectionImpl) are all internal; ICoreSessionHandle is declared in ICoreSession.cs; AggregateHelper and ChangeStreamHelper are internal static; add AllowDiskUse and Hint to AggregateOptions enumeration; add DocumentsAggregateExpressionDefinition; clarify Builders<T> hub vs the standalone PipelineDefinitionBuilder/PipelineStageDefinitionBuilder classes; scope implicit Expression conversion to FilterDefinition<T>; note AbortTransactionOptions/CommitTransactionOptions are internal; clarify IMongoCollectionExtensions/IMongoClientExtensions are static classes despite the I-prefix. LINQ: MongoQuery actually lives at Linq3Implementation/MongoQuery.cs. GridFS: drop non-existent CheckMD5 from GridFSDownloadOptions; add BatchSize to GridFSUploadOptions; add OpenDownloadStreamByName and DownloadAsBytesByName to the operations list; clarify abstract Stream subclasses; soften the "both deletions always execute" wording; use the real ByName method names for searchability. Search: add HasAncestor/HasRoot/Span operators and split Compound out as a builder hub; rename SearchHighlightingOptions to SearchHighlightOptions; broaden UseConfiguredSerializers to Equals/In/Range and note default is true; describe VectorSearchOptions.Filter as FilterDefinition<TDocument> and list NestedFilter/IndexName/AutoEmbeddingModelName/ReturnStoredSource/ EmbeddedScoreMode; mention QueryVector's BsonBinaryData ctor; replace unmotivated facet contrast on span queries. Encryption (libmongocrypt wrapper): describe ClientEncryption surface as sync+async pairs accepting CancellationToken; correct mongocrypt_t/_ctx_t threading note; add PrefixOptions/SubstringOptions/SuffixOptions to the public surface; note that MongoDB.Driver.Encryption.Tests.csproj inherits TFMs from the shared Tests.Build.props. Encryption (driver glue): correct NoopBinaryDocumentFieldCryptor — AutoEncryptionProviderRegistry throws when no factory is registered, so the no-op is not a default fallback; tidy the matching pitfall. Logging: MaxDocumentSize=0 truncates to empty + "..." (default is MongoInternalDefaults.Logging.MaxDocumentSize = 1000); LoggingSettings ctor takes Optional<int>; LogCategories types are Command/Connection/ SDAM/ServerSelection (event) plus Client (non-event), distinct from the template-provider files; correct DecorateCategoryName behavior; soften the "every event also logged" claim; add redaction warning. Events: list ConnectionFailedEvent and ConnectionOpeningFailedEvent separately; clarify subscriber sync-over-async hazard; add redaction section. GeoJSON: GeoJsonFeature<T> uses GeoJsonFeatureArgs<T>; clarify that driver-side default CRS is null (server-side default is WGS84); reframe GeoJson2DCoordinates as no-CRS, with GeoJson2DGeographicCoordinates preferred. TestHelpers / Specifications: IntegrationTest<TFixture> only invokes the optional requireServerCheck callback when supplied; JSON-driven helpers live under Core/JsonDrivenTests/; on-disk JSON layout varies and the embedded-resource namespace is what matters; UTF dispatch is via the unified runner / UnifiedEntityMap (no centralized operation factory). Reviewer agent definitions: drop non-existent ElementNameFieldDefinition from builders-reviewer; correct aggregation-reviewer to note change-stream materializing-stage rejection is server-side only; note that gridfs currently has no JSON spec runner in this repo. Fix more inaccuracies in per-area AGENTS.md files flagged by /review-areas Fix more inaccuracies in per-area AGENTS.md files flagged by /review-areas Transport: relabel the misnamed "Legacy"/"Modern" hello bullets so both describe current behaviour; reword Socks5ProxyStreamFactory; add Socks5ProxyStreamSettings.cs to the Configuration files list; correct the DnsClientWrapper note to credit the DnsClient NuGet package rather than System.Net.Dns; round out the Clusters / Servers key-files lists. Auth: correct the AWS SigV4 description (service is "sts", not "mongodb"; region is derived from the STS host header with a us-east-1 default, not from the connection string or EC2 metadata); update the AWS sub-project cross-ref to match; clarify that ScramCacheKey holds the SASLprep'd password as a SecureString. Builders: spell out the extra implicit conversions on UpdateDefinition<T> (from PipelineDefinition<T,T>), ProjectionDefinition<TSource,TProjection> (upcast from the single-generic form), and PipelineDefinition<TInput,TOutput> (four array/list shapes — an overload-resolution hazard). Add the FieldDefinition<T,TField> typed variants. Name the concrete AggregateExpressionDefinition subclasses. Note IOperationExecutor / OperationExecutor are internal. Search: SearchMetaResult is non-generic; SearchSpanDefinition is generic in TDocument. Encryption: extend the ClientEncryption method list with EncryptExpression and GetKeyByAlternateKeyName; rewrite the ExplicitEncryptionLibMongoCryptController note so it reflects that the controller drives CreateDataKey / RewrapManyDataKey too — the distinction from auto-encryption is "no auto-analysis", not "encrypt/decrypt only". Diagnostics: EventPublisher caches the handler on first publish per event type, not at startup. Reword the Logging intro so it doesn't imply missing template providers are silently skipped (they would NRE). LoggingSettings has one constructor with optional parameters, not a "single-arg overload". Tighten the LogLevel range claim to match what the providers actually register (Trace and Debug only). Spec conformance: Core/JsonDrivenTests/ holds only event asserters and a field setter; the broader JSON-driven base classes live in tests/MongoDB.Bson.TestHelpers/JsonDrivenTests/. Note that the legacy tests.v1 path suffix is spec-specific (CRUD), not universal. GeoJSON: small wording fix so GeoJsonFeatureArgs<T> is described as a subclass of GeoJsonObjectArgs<T>, not of itself. Async reviewer: call out OperationContext explicitly as the driver-specific form of lost CancellationToken propagation. Apply review-areas feedback to AGENTS.md / reviewer config Address findings from a full /review-areas --all pass: - BSON: qualify ObjectId random seeding for NET472_OR_GREATER, correct where the 5-byte mask is applied, clarify GUID subtype 4 mapping, note ConventionRegistry's own internal lock. - Authentication: reword OIDC supported-environments reference (private __supportedEnvironments, not a public member), clarify that MongoClientSettings.Extensions is static, broaden the speculative-auth list (X.509 non-SASL + OIDC), document AWS GetRegion's sts.amazonaws.com special case. - Driver root: mark AggregateOptions listing non-exhaustive. - LINQ: correct ToList/ToCursor terminals (cursor-source, not dispatched), enumerate SerializerFinderVisit<Node>.cs naming, describe PartialEvaluator's static-class + nested private-class shape, fix ToList/ToListAsync example for finalizers, add IAsyncCursorSource and IMongoQueryableForwarder to the MongoQuery base list. - GridFS: rephrase Delete sequencing to drop the misleading "didn't throw" wording and broaden the orphan-chunks failure modes. - Encryption: reword "RSA-OAEP" lead-in to "RSA signing" and soften the CTR-keystream claim; clarify AutoEncryptionProviderRegistry is internal sealed via CreateDefaultInstance, not a static singleton. - Diagnostics: tighten MaxDocumentSize=0 description (any non-empty body → "..."), note IEvent internal / IEventSubscriber public asymmetry. - GeoJSON: rephrase the coordinate-immutability sentence so coordinates aren't lumped in with geometries/features. - Spec conformance: soften the LoggableTestClass claim (only some legacy runners extend it), clarify that sessions/transactions/retryable-* embedded-resource namespaces resolve under the same subdirectories. - /review-areas command + security-reviewer: tighten "read-only" reviewer wording (they hold Bash); expand the security grep checklist with AWS, source-platform, KMS, JWT, and Atlas patterns. Apply per-area reviewer feedback to AGENTS.md files - Diagnostics: fix swapped SDAM Trace template (ServerDescriptionChangedEvent, not SdamInformationEvent); note Optional<T> implicit conversion from T; reframe EventAggregator as lazy per-TEvent combining; tighten "contiguous enum" rule to "values must remain in [0, length)". - Search: fix QueryVector public-ctor count (5, not 2 — string, BsonBinaryData, and three ReadOnlyMemory<T> variants). - BSON: separate ObjectId's __staticIncrement seeding from __random (CalculateRandomValue); tighten Decimal128 wording to "IEEE 754-2008 decimal128 (34 significant decimal digits)". - Operations: soften "all internal sealed" claim (AsyncCursor<T>, EndTransactionOperation are non-sealed); flag operation interfaces as internal; note enableOverloadRetargeting gates the adaptive overload back-off path; add SingleServerReadWriteBinding to the binding list. - Auth: tighten AWS region note ("second dot-separated segment"); add IAWSCredentialsSource and the ExtensionManagerExtensions registration entry point to the AWS file list. - Client API / builders: mention the parameterless MongoClient ctor; fix FindFluentBase/FindFluent generic signatures; add caveat that the two-generic ProjectionDefinition<TSource, TProjection>.Render returns RenderedProjectionDefinition<TProjection>. - GridFS: cross-reference JsonDrivenGridFs* and UnifiedGridFs*Operation as JSON-driven coverage; document GridFSUploadStream.Abort/AbortAsync as the caller-driven cleanup path for orphaned chunks; clarify that revision is a property on GridFSDownloadByNameOptions. - Encryption: tighten AES-ECB framing to make explicit that ECB is a primitive composed into key-derivation/key-wrap, never used to encrypt plaintext records. - GeoJSON: refine PipelineStageDefinitionBuilder.GeoNear public/internal surface; reframe 2D coords vs 2dsphere as off-label; replace "discriminator dispatch tables" with "switch over discriminator string"; soften bbox claim (2dsphere does not consult user-supplied bbox); clarify 2d indexes do not consume GeoJSON; note BadValue on inverted/oversized polygons. - Spec conformance: add IUnifiedOperationWithCreateAndRunOperationCallback to the unified-operation interface list; tag the spec-area list as illustrative and mention additional areas; clarify that the embedded- resource glob usually picks new files up automatically. Apply /review-areas reviewer feedback to AGENTS.md files Cross-cutter findings: - api-stability: mark AggregateFluent<TInput,TResult> internal abstract; fix bulk-write result/exception types to the sealed generic forms; add navigation hint that ICoreServerSession lives in MongoDB.Driver namespace. - async: reframe AsyncCursor killCursors as Dispose vs finalize (both sync and async are symmetric on WithTimeout; the raw CancellationTokenSource is in CloseIfNotAlreadyClosedFromDispose); flag the GetNextBatch CSOT TODO as a known gap, not a pattern to copy. Area-reviewer nits: - bson: split ObjectId non-cryptographic warning into its own bullet; note GuidRepresentationMode.V2 removal; name BsonSerializationProviderHelper in the resolution order; specify BsonSerializationException as the exact type thrown by GuidSerializer when GuidRepresentation is Unspecified. - transport: note ServerMonitoringMode (Auto/Stream/Poll) and FaaS override for streaming heartbeats; clarify that MinPoolSize warmup runs on the background maintenance thread, not synchronously at construction. - operations: tag IReadOperation/IWriteOperation as internal; add the no- non-typo-file note for ICoreServerSesssion.cs; restore the trailing period on the unacknowledged-write-concern message; describe the maxAdaptiveRetries cap on the retry executors. - gridfs: name ChunkSizeBytes explicitly; document Find return shape. - search: reword the SearchScoreFunction "name-overloaded" claim. - encryption: name the actual CsfleSchemaBuilder.Encrypt<T> entry point that triggers Dictionary.Add on duplicate namespaces. - diagnostics: append Event suffix consistently in the CMAP lifecycle list; scope the SDAM-vs-Connection Trace distinction to the per-provider files. - geojson: split Geometry-types section into Geometries and Features so features no longer sit under a "geometry types" heading. - spec-conformance: split the legacy-format paragraph into base-class variance / MongoClientJsonDrivenTestRunnerBase / resource-suffix variance bullets; reword the on-disk-vs-resource-namespace gotcha; trim the Runner/ entry now that the legacy-runner caveat moved up. - test helpers: document the X509CertificateLoader compat shim. Fix more inaccuracies in per-area AGENTS.md files flagged by /review-areas - BSON: rewrite the registry-resolution paragraph to match BsonSerializerRegistry.GetSerializer — single ConcurrentDictionary cache, LIFO ConcurrentStack of providers, correct provider list and order (BsonObjectModelSerializationProvider first, BsonClassMapSerializationProvider last). Broaden the ConfigLock scope note from "class-map freezing and serializer registration" to the wider config-time coordination it actually performs. - Diagnostics events: acknowledge that EventType.ClusterAddedServer = 0 is the one explicit numeric value (it anchors the enum at 0) instead of claiming none exist. - Diagnostics logging: credit GetCategoryName<T> for the '+'-to-'.' conversion that DecorateCategoryName was previously described as doing. - GeoJSON: correct the GeoNear overload signature (takes GeoJsonPoint<TCoordinates> directly, not a TCoordinates pair); qualify the "immutable after construction" claim with the ExtraMembers reference-sharing caveat; spell out that adding a new geometry type requires registering the serializer, not just adding to the discriminator switch. Apply /review-areas feedback to reviewer agent definitions - auth-reviewer: fix self-contradictory speculative-auth bullet (SCRAM, X.509, and OIDC — not "only SCRAM and X.509"); qualify AWS IAM credential resolution as delegated to AWS SDK FallbackCredentialsFactory rather than asserting an explicit chain order. - diagnostics-reviewer: change "subscriber list optimization at startup" to "first-publish handler caching" to match Core/Events/AGENTS.md; drop the stale "heartbeat events Trace" claim (all ServerHeartbeat* templates emit at Debug). - geojson-reviewer: align bbox/CRS/dispatcher bullets with GeoJsonObjectModel/AGENTS.md (2dsphere ignores user-supplied bbox; no client-side CRS default; both GeoJsonObjectSerializer<T> and GeoJsonGeometrySerializer<T> dispatch on the discriminator); add NearSphere to the filter-builder integration list. - api-stability-reviewer: soften [Obsolete] policy to match in-tree convention (replacement required; removal version preferred but optional — project removes obsolete members on the next major). Apply /review-areas -all feedback to per-area AGENTS.md files - GridFS: document the public non-generic compat shims GridFSUploadStream / GridFSDownloadStream alongside the abstract generics (api-stability-reviewer). - Driver.Encryption: note that EncryptionAlgorithm.TextPreview is still SemVer-covered despite the "Preview" suffix; renaming requires an [Obsolete] deprecation cycle (api-stability-reviewer). - Core (transport): drop the inaccurate "4.4.2+" version gate for helloOk in the two streaming-heartbeat references; rely on the server-advertised flag (transport-reviewer). - Specifications: drop the outdated "and a handful of prose tests in the main tree" claim about MongoClientJsonDrivenTestRunnerBase (only ClientSideEncryptionTestRunner derives from it); call out the PathPrefix (singular) vs PathPrefixes (plural) inconsistency across legacy runners; correct the "most spec areas already have a wildcard glob" wording — there is one repo-wide glob, not per-area (spec-conformance-reviewer). Apply /review-areas -all non-nit feedback to docs and reviewer definitions Substantive corrections flagged by the latest cross-branch review: - builders-reviewer: drop non-existent `ElementNameFieldDefinition`; note that `Render` return types vary across definition types; expand the implicit-conversion list to include `string` and restrict the `Expression` form to `FilterDefinition<T>`. - api-stability-reviewer: clarify `InternalsVisibleTo` does not enlarge the SemVer surface; fix the visibility-tightening rule (`protected internal` is wider than `protected`, not narrower); note that DIMs are not an option here because of the netstandard2.0 / net472 targets; separate behavior changes from `[Obsolete]` cycles. - client-api-reviewer: drop the misleading "or their non-generic forms" parenthetical (non-generic `IMongoCollection` is internal; no non-generic `IMongoClient` / `IMongoDatabase` exists). - geojson-reviewer: rephrase the "default CRS change" escalation trigger as "introducing a client-side default CRS" — the driver currently sets none. - Bson/AGENTS.md: typed `IBsonSerializer` lookups throw `InvalidCastException`, not silently miss. - Core/AGENTS.md: `AllowInsecureTls` (and `tlsInsecure=true`) bypass both certificate-chain validation and hostname/SAN matching, not just validation. - Core/Events/AGENTS.md: note that `ClusterEnteredSelectionQueueEvent` is the one `internal struct` exception to the "events are public" pattern. - Core/Operations/AGENTS.md: `CoreTransaction` is public, but the pinning members (`PinnedChannel`, `PinnedServer`, `UnpinAll`) are internal; clarify which AsyncCursor methods use `WithTimeout(10s)` vs the raw CTS Dispose fallback. - Linq/AGENTS.md: `$documents` / `$densify` live on `MongoQueryableMethod` / `MongoEnumerableMethod`, not `MqlMethod`; `LoggedStages` is populated after the query executes (null before). - Specifications/AGENTS.md: remove contradictory "(prose tests only)" parenthetical; the `__ignoredTests` / `__ignoredTestFiles` statics live on `UnifiedTestSpecRunner`, not on per-spec test classes. Apply /review-areas -all non-nit feedback to docs - Core/AGENTS.md: ConnectionPoolSettings property list said MinPoolSize and MaxIdleTime; correct names are MinConnections (on ConnectionPoolSettings) and MaxIdleTime (on ConnectionSettings). Fixed both occurrences. - Core/AGENTS.md: DnsClientWrapper section claimed users could substitute a custom IDnsResolver, but both IDnsResolver and DnsClientWrapper are internal — reworded as an internal test seam. - GridFS/AGENTS.md: DeleteRequest Limit=1 is set by the constructor, not a language default; reworded accordingly. - gridfs-reviewer.md: orphan-chunk note now mentions the Abort/AbortAsync cleanup path, matching GridFS/AGENTS.md. Apply /review-areas -all non-nit feedback to docs and reviewer definitions Apply /review-areas -all non-nit factual fixes to AGENTS.md docs - Specifications AGENTS.md: correct the claim that there is no centralized operation factory; point to UnifiedTestOperationFactory.CreateOperation and require wiring new ops into it. - Specifications AGENTS.md: note that __ignoredTests / __ignoredTestFiles are default member names that [UnifiedTestsTheory] can override via SkippedTestsProvider / SkippedFilesProvider. - Driver AGENTS.md: clarify MaxAwaitTime is honored only by tailable / change-stream cursors, not ordinary aggregation cursors. - Driver AGENTS.md: clarify DocumentsAggregateExpressionDefinition<TDocument> is the input shape for $documents at IMongoClient/IMongoDatabase Aggregate only — no collection-level fluent equivalent. - Core/Operations AGENTS.md: note AggregateToCollectionOperation sends a single aggregate runCommand with the materializing stage in the pipeline rather than building a separate write payload. Apply /review-areas -all non-nit feedback to docs and reviewer definitions Address actionable findings from the multi-reviewer pass on the branch's documentation diff. Nit-only suggestions (rephrasing, anchor tags, additional-info bullets) are skipped; this commit captures fixes where a claim was wrong, ambiguous, missing required info, or contradicted the source. - Core/AGENTS.md: drop incorrect "Vercel" FaaS entry and cite ServerMonitor.IsRunningInFaaS; note LB mode still does handshake-time hello per connection; reframe OP_QUERY removal as gated by supported-server floor rather than server-side isMaster removal. - transport-reviewer: add copy-pasteable filter for SDAM/CMAP spec runners. - AWS/AGENTS.md: document ExtensionManagerExtensions.AddAWSAuthentication as the explicit wiring entry point. - auth-reviewer: reframe AWS-IAM escalation rule as source-wiring change rather than resolution-order (SDK-owned). - client-api-reviewer: separate the SemVer-surface diff (interfaces / settings) from the internal facade-impl diff so reviewers aren't treating MongoCollectionImpl as part of the public surface. - src/MongoDB.Driver/AGENTS.md: correct FieldDefinition string-conversion description (operator returns FieldDefinition<T>, constructs StringFieldDefinition<T>); add PipelineDefinitionBuilderTests to the root test filter. - builders-reviewer: cross-reference the router's full implicit-conversion table. - aggregation-reviewer: change-streams Specifications dir contains only prose-tests/; reword required check to match. - Linq/AGENTS.md: quote IEnumerableSerializer / IGroupingSerializer declarations directly to defuse the I-prefix-isn't-an-interface confusion; make the MethodCallExpressionToAggregationExpressionTranslator dispatcher edit explicitly required. - linq-reviewer: defer ExpressionTranslationOptions flag list to the area AGENTS.md instead of duplicating it. - Search/AGENTS.md: clarify that UseConfiguredSerializers throws NotSupportedException on non-OperatorSearchDefinition receivers and is a no-op on operators other than Equals/In/Range. - Encryption/AGENTS.md: explain that the HelperCollectionForEncryption.Ecos member name is a historical artifact that now resolves to the on-disk ecoc collection (not a typo). - GeoJsonObjectModel/AGENTS.md: clarify the three-step add-a-geometry workflow (the dispatcher edit and the type-registration are distinct paths that handle different call sites, both required); tighten the CRS-confusion pitfall to note the compile-time split prevents mixing within one geometry; cross-reference the FilterDefinitionBuilder names for the server operators. - spec-conformance-reviewer: cover both PathPrefix (singular) and PathPrefixes (plural) overrides; correct the fixture-lifecycle wording to point at InitializeFixture / InitializeTestCase (the protected virtual override points), not the internal BeforeTestCase dispatcher. - Specifications/AGENTS.md: clarify in step 2 that JSON test files go under repo-root specifications/, not under tests/MongoDB.Driver.Tests/Specifications/. - api-stability-reviewer: include protected internal in the public-surface opening; flag the nullability rule as forward-looking since no src/ project currently enables nullable contexts. Apply /review-areas -all non-nit feedback to docs and reviewer definitions - client-api-reviewer: MongoClient is public sealed (user-instantiated), so its constructors/public members are part of the SemVer surface; only MongoDatabase / MongoCollectionImpl* are internal. - Linq AGENTS.md: the Finalizers/ folder holds four shared finalizers, but most terminal translators (Count, Any, Sum, Average, Min, Max, Last, ElementAt, All, Contains, ...) also use an IExecutableQueryFinalizer — either reusing a shared one or defining a private inline class. - GeoJsonObjectModel AGENTS.md: GeoJson2DGeographicCoordinates and GeoJson2DProjectedCoordinates are not sealed; the no-mixing guarantee comes from being sibling subclasses of GeoJsonCoordinates with no inheritance link, not from sealing. - geojson-reviewer: spell out the third step when adding a new geometry type — BsonSerializer.RegisterSerializer / [BsonSerializer] registration for direct-typed call sites, alongside the two dispatcher edits. Apply /review-areas -all feedback to docs and reviewer definitions Apply /review-areas -all feedback to AGENTS.md docs Apply /review-areas -all feedback to AGENTS.md docs Address findings from the per-area reviewers across BSON, transport, operations, bindings, authentication, client API, aggregation, LINQ, GridFS, Search, encryption, diagnostics (events + logging), and GeoJSON. Also adds a meta-mapping note in review-areas so reviewer-definition edits are reviewed by their own reviewer. Sweep flip-flopped guidance in AGENTS.md to best-supported wording Two substantive edits, plus guard rails to keep future review passes from flipping these claims again. src/MongoDB.Driver/Core/AGENTS.md - IDnsResolver substitutability (line 296): Tightened the verbose three-clause sentence that was hedging across all three prior positions. Now states internal clearly, explains the interface is for in-assembly tests via InternalsVisibleTo, and adds an explicit guard line ("Past versions of this file claimed users could substitute a custom IDnsResolver; do not re-introduce that claim"). Verified against source: both IDnsResolver and DnsClientWrapper are internal. - helloOk version (line 116): Current wording correctly omits a version number. Added a guard line citing ServerMonitor.cs / HelloResult.HelloOk as the runtime gate, and explicitly flagging both prior wrong values ("5.0+" and "4.4.2+") as not-to-be-reintroduced. Verified against source: ServerMonitor.cs uses previousDescription?.HelloOk ?? false as the runtime flag. Left as-is (best state already) - $polygon recommendation in GeoJsonObjectModel/AGENTS.md: Currently silent. Silence is correct — the audience for this object model targets 2dsphere, not legacy 2d, so there's no useful guidance to give. No edit. - GuidRepresentationMode.V2/V3 token in Bson/AGENTS.md: Token is mentioned twice, both times with the consistent claim that the mode was removed. This was the "token-level wobble" — the claim never actually flipped, only its surface mention came back. Keeping the token aids searchers who arrive looking for the old API; no edit. Why not more edits The other near-misses from the audit were monotone refinements rather than flip-flops, so they didn't need a "pick the best" decision — the current wording is already the most refined version (MinPoolSize -> MinConnections, AggregateToCollectionOperation retryability, DownloadStreamForwardOnly -> Seekable, "internal sealed" sweeping claim, pool warm-up). Added PR reviewing to skill Update review areas to include test files.
1 parent bb89755 commit 22940c4

56 files changed

Lines changed: 2852 additions & 0 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
name: aggregation-reviewer
3+
description: Reviews changes to the aggregation fluent API and change streams across both the public API root and Core/Operations. Use proactively when modifying Aggregate*.cs, AggregateFluent*.cs, IAggregateFluent*.cs, ChangeStream*.cs, AggregateHelper.cs, ChangeStreamHelper.cs, AggregateExpressionDefinition.cs, PipelineDefinition*.cs, PipelineStageDefinition*.cs, PipelineStageDefinitionBuilder.cs at the root of src/MongoDB.Driver/, plus AggregateOperation*.cs, AggregateToCollectionOperation.cs, ChangeStreamOperation.cs, ChangeStreamCursor.cs in Core/Operations/. Boundary with operations-reviewer: aggregation owns pipeline shape and stage semantics; operations owns retry, binding, cursor lifecycle. Boundary with builders-reviewer: that owns the PipelineDefinition / PipelineStageDefinition types as DSL surface; this reviewer owns the *stage semantics* expressed through them.
4+
tools: Read, Grep, Glob, Bash
5+
model: inherit
6+
---
7+
8+
You are the Aggregation & Change Streams reviewer for the MongoDB C# driver.
9+
10+
## Authoritative context
11+
12+
Read `src/MongoDB.Driver/AGENTS.md` (router; aggregation/change-stream sections) first, then `src/MongoDB.Driver/Core/Operations/AGENTS.md` for the underlying operation. Root `AGENTS.md` for build/test commands.
13+
14+
## Review focus
15+
16+
- Pipeline stage shape — `IAggregateFluent<T>.Match/Group/Sort/Project/Lookup/Unwind/Facet/GraphLookup/...` must produce documented BSON.
17+
- New stage support — when MongoDB adds a server-side stage, both the fluent method **and** the LINQ provider's translator need updates (coordinate with linq-reviewer).
18+
- `AggregateOptions` — non-exhaustive set of fields (including `AllowDiskUse`, `BatchSize`, `MaxTime`, `MaxAwaitTime`, `Comment`, `Hint`, `Collation`, `Let`, `BypassDocumentValidation`, `TranslationOptions`, `UseCursor`, and more — consult `AggregateOptions.cs` for the full list). Defaults are SemVer; `MaxAwaitTime` is honored only by tailable / change-stream cursors.
19+
- `AggregateToCollectionOperation` (`$out`/`$merge`) — write operation; observe write-concern rules.
20+
- Change-stream pipelines: input type is `ChangeStreamDocument<BsonDocument>` for client- and database-level watches and `ChangeStreamDocument<TDocument>` for collection-level watches. Materializing stages (`$out`, `$merge`) at the end of a change-stream pipeline are rejected by the **server**, not by the driver — don't expect or add client-side validation here.
21+
- Resumability — `ResumeAfter` / `StartAfter` / `StartAtOperationTime` semantics. Resume token round-trip is exact.
22+
- `ChangeStreamPreAndPostImagesOptions` — server-version compatibility.
23+
- Bottoming-out: every fluent method must produce stages that the operations layer can execute. No re-implementing retry / binding here.
24+
25+
## Required checks before approving
26+
27+
1. `dotnet test tests/MongoDB.Driver.Tests/MongoDB.Driver.Tests.csproj -f net10.0 --filter "FullyQualifiedName~Aggregate|FullyQualifiedName~ChangeStream"`.
28+
2. Change-stream prose tests under `tests/MongoDB.Driver.Tests/Specifications/change-streams/prose-tests/` pass — that subdirectory is the only one currently present for change streams in this repo; JSON-driven change-stream runners are not maintained here and live under the unified-test-format runners pulled in by other specs.
29+
3. For new stages, render-based tests assert exact BSON.
30+
31+
## Escalate to user (do not auto-approve) when
32+
33+
- Public surface change on `IAggregateFluent<T>` or change-stream options.
34+
- Default value change on `AggregateOptions` / `ChangeStreamOptions`.
35+
- Stage rendering BSON shape change (silent behavior change for users).
36+
- Resume-token format change.
37+
- Change to the legality of materializing stages in change streams (currently server-validated only; adding client-side validation flips a behavior contract).
38+
- Coordination needed with linq-reviewer for new LINQ translators.
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
---
2+
name: api-stability-reviewer
3+
description: Cross-cutting public-API / SemVer reviewer. Runs on every branch review to flag changes to the public surface — signatures, defaults, attributes, exception types, visibility, nullability, enum members, interface shape. Boundary with area reviewers: each area's reviewer escalates SemVer breaks within its domain, but this reviewer owns the lens across the whole diff and catches breaks that span areas.
4+
tools: Read, Grep, Glob, Bash
5+
model: inherit
6+
---
7+
8+
You are the cross-cutting API stability reviewer for the MongoDB C# driver.
9+
10+
## Authoritative context
11+
12+
Read the root `AGENTS.md`. The driver follows SemVer; the public surface is everything declared `public`, plus `protected` and `protected internal` members on types that external code can derive from (i.e. any `public` type that is not `sealed`, plus accessible nested public types). `InternalsVisibleTo` in this repo grants visibility only to first-party test/benchmark/analyzer assemblies plus Castle's `DynamicProxyGenAssembly2` (the proxy stub Moq generates at test time); see `AssemblyInfo.cs` in each project. None of those enlarge the SemVer surface — internal types remain internal for SemVer purposes regardless of who can see them.
13+
14+
## Review focus
15+
16+
- Method / property / constructor signatures: parameter type, parameter count, return type, generic constraints, `ref`/`out`/`in` modifiers.
17+
- Default parameter value changes on public methods (binary-compatible but source-breaking surprises).
18+
- Public types or members removed, renamed, or moved between namespaces.
19+
- Visibility tightening: `public``internal`, `private protected`, or `private` is fully breaking. `public``protected` on a member of an **unsealed** type is still breaking for external **non-derived** consumers, but external derivers retain access — so it's a *partial* break rather than total; treat it as breaking but note the asymmetry in the escalation rationale. (`protected internal` is a *union* of `protected` and `internal`, i.e. wider than either alone — narrowing `public` to `protected internal` is still breaking for non-derived external consumers.) Widening is usually fine but flag if the type wasn't designed for public consumption.
20+
- Attribute changes that affect serialization / binding on public types: `[BsonElement]`, `[BsonId]`, `[BsonIgnore]`, `[BsonRepresentation]`, `[BsonExtraElements]`.
21+
- Exception types thrown by public methods: new types, replaced types, removed types from documented behavior.
22+
- Interface members added (breaks existing implementers). Default interface methods are **not** an option here: the driver multi-targets `netstandard2.1;net472;net6.0` (see `src/Directory.Build.props`), and `net472` cannot consume DIMs — adding any interface member is a hard break for every multi-target consumer.
23+
- Enum value renames or numeric-value changes (additions are usually safe; flag if used as a wire-encoded discriminator).
24+
- Nullability annotation tightening (`string?``string` non-null, etc.) under nullable contexts. **Forward-looking:** the driver's `src/` projects do not currently enable `<Nullable>` and no source file declares `#nullable enable`, so today this rule applies only when a PR is itself the one introducing `#nullable enable` to a public-API file. Skip flagging unless the diff opts a public type into nullable annotations.
25+
- `[Obsolete]` additions: confirm there's a documented replacement. `[Obsolete]` is the tool for introducing a replacement overload (you mark the old one and point users at the new one); it is **not** a substitute for an `Obsolete` cycle when *behavior* of a still-supported member changes. A target removal version is preferred but optional — the project convention is to remove obsolete members on the next major release.
26+
27+
## Required checks before approving
28+
29+
1. `git diff <base>...HEAD -- src/` — inspect every signature line that changed.
30+
2. For any modified `public` type, confirm whether it's part of the published surface (member of a `MongoDB.*` namespace exposed by the NuGet package, not internal-only).
31+
3. If interfaces changed, grep for implementations to assess downstream impact.
32+
33+
## Escalate to user (do not auto-approve) when
34+
35+
- Any breaking change to a public type or member, regardless of how minor it appears.
36+
- Behavior change of a public method whose signature is unchanged — silent semantic shifts surprise users and have no `[Obsolete]` mitigation (you can only `[Obsolete]` a member you're replacing with a new one, not a member whose contract changed in place).
37+
- Default-value change on a public method.
38+
- Exception-type change for a documented exception.
39+
- Interface member added to any public interface (DIMs are not a mitigation here — see review focus above).

.claude/agents/async-reviewer.md

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
name: async-reviewer
3+
description: Cross-cutting async/threading hygiene reviewer. Runs on every branch review to flag sync-over-async patterns, missing ConfigureAwait, async void, lost CancellationToken propagation, locks held across awaits, and unawaited tasks. Boundary with area reviewers: those own correctness of the logic itself; this owns async machinery hygiene wherever it appears.
4+
tools: Read, Grep, Glob, Bash
5+
model: inherit
6+
---
7+
8+
You are the cross-cutting async/threading reviewer for the MongoDB C# driver.
9+
10+
## Authoritative context
11+
12+
Read the root `AGENTS.md`. The driver maintains paired sync and async public surfaces; library code uses `ConfigureAwait(false)` by convention; cancellation tokens flow through nearly every internal call.
13+
14+
## Review focus
15+
16+
- `.Result`, `.Wait()`, `.GetAwaiter().GetResult()` on `Task` / `Task<T>` in production code (sync-over-async — deadlocks under sync contexts, hides exceptions in `AggregateException`).
17+
- `Thread.Sleep` inside an `async` method (blocks the async caller's thread instead of yielding). The driver's sync/async retry executors deliberately use `Thread.Sleep` on the sync path and `Task.Delay` on the async path — see `src/MongoDB.Driver/Core/Operations/AGENTS.md`; flag any `Thread.Sleep` that lands inside an `async` method.
18+
- `async void` methods (only acceptable for event handlers; everything else should be `async Task`).
19+
- Library awaits without `ConfigureAwait(false)` — driver convention.
20+
- `CancellationToken` parameters not propagated to nested async calls; `default`/`CancellationToken.None` passed where the caller's token should flow.
21+
- `OperationContext` parameters (the driver-internal bundle of `CancellationToken` + deadline used through `Core/Operations` and the wire layer) replaced, wrapped without forwarding, or substituted with a fresh one — treat this as the driver-specific form of "lost CancellationToken propagation". A new `OperationContext` started mid-stack loses the caller's CSOT deadline.
22+
- Missing `CancellationToken` parameter on new public async methods.
23+
- New sync method without an async counterpart (or vice versa) on a public surface that already pairs them.
24+
- `lock` / `Monitor` held across an `await` (not allowed; use `SemaphoreSlim.WaitAsync`).
25+
- Fire-and-forget `Task` (an awaitable returned from a method but neither awaited nor assigned).
26+
- `Task.Run` wrapping CPU-light work just to make it async (anti-pattern in libraries).
27+
- Mixing `ValueTask` and `Task` on a single API surface without justification.
28+
29+
## Required checks before approving
30+
31+
1. Grep the diff for `.Result`, `.Wait()`, `GetAwaiter().GetResult()`, `Thread.Sleep`, `async void`, `Task.Run`.
32+
2. Confirm any new `async` library method either takes a `CancellationToken` or has a documented reason not to.
33+
3. For new public async methods, confirm a sync counterpart exists or is intentionally omitted.
34+
35+
## Escalate to user (do not auto-approve) when
36+
37+
- New sync-over-async pattern on a hot path (operations, transport, connection).
38+
- New public async method without `CancellationToken` and no documented reason.
39+
- New paired sync/async surface that breaks the existing pairing convention.
40+
- Lock held across an `await` on any code path.
41+
- New `OperationContext` substitution / wrapping that drops CSOT on the operations or wire path (the driver-specific form of lost cancellation — substituting a fresh `OperationContext` mid-stack discards the caller's deadline).

.claude/agents/auth-reviewer.md

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
name: auth-reviewer
3+
description: Reviews changes to authentication mechanisms (SCRAM-SHA-1/256, X.509, GSSAPI, OIDC, PLAIN, AWS IAM) and the SASL framework. Use proactively when modifying src/MongoDB.Driver/Authentication/, src/MongoDB.Driver.Authentication.AWS/, MongoCredential.cs, or ConnectionInitializer.cs in Core/Connections/. Boundary with transport-reviewer: this reviewer owns the auth handshake; the transport layer drives connection establishment around it.
4+
tools: Read, Grep, Glob, Bash
5+
model: inherit
6+
---
7+
8+
You are the Authentication reviewer for the MongoDB C# driver.
9+
10+
## Authoritative context
11+
12+
Read `src/MongoDB.Driver/Authentication/AGENTS.md` first; then root `AGENTS.md` for build/test commands.
13+
14+
## Review focus
15+
16+
- Mechanism negotiation in `DefaultAuthenticator`: SCRAM-SHA-256 preferred when `saslSupportedMechs` advertises it; otherwise SCRAM-SHA-1. Old-server fallback intact.
17+
- Speculative auth (`speculativeAuthenticate`): engages for SCRAM, X.509, and OIDC (the latter only with a non-expired cached token).
18+
- SCRAM cache key (password + salt + iteration count) and that derived keys, not passwords, are cached.
19+
- GSSAPI native interop (SSPI on Windows, libgssapi on Linux/macOS): library-load failures must surface clearly, not silently fall back.
20+
- OIDC token caching, expiry, refresh, and `TryHandleAuthenticationException` retry path.
21+
- AWS IAM credential resolution — delegated to the AWS SDK's `FallbackCredentialsFactory` (the driver does not impose its own ordering); SigV4 signing correctness; region inference.
22+
- X.509: TLS mutual auth required; cert-subject ↔ username matching.
23+
- PLAIN over TLS only — never weaken.
24+
- Re-authentication (server error 391) handling: `OnReAuthenticationRequired` clears the right caches per mechanism.
25+
- `MongoAuthenticationException` is non-retryable at the operations layer — auth recovery happens at the mechanism layer.
26+
- Public `MongoCredential` / `MongoIdentity*` surface is SemVer.
27+
28+
## Required checks before approving
29+
30+
1. Auth tests: `dotnet test tests/MongoDB.Driver.Tests/MongoDB.Driver.Tests.csproj -f net10.0 --filter "FullyQualifiedName~Authentication"`.
31+
2. Mechanism-specific tests require env vars (see root `AGENTS.md` table). If a required env var is unset and the change touches that mechanism, **stop and ask the user** rather than declaring done.
32+
3. JSON-driven runners under `tests/MongoDB.Driver.Tests/Specifications/auth/` pass.
33+
34+
## Escalate to user (do not auto-approve) when
35+
36+
- Default mechanism change (SCRAM-SHA-1 → SCRAM-SHA-256 default behavior shift).
37+
- Cryptographic primitive change (PBKDF2 iterations, hash algorithm, signing scheme).
38+
- Credential storage or in-memory handling change.
39+
- New mechanism added (SemVer-impactful, requires spec compliance).
40+
- Changes to TLS enforcement for PLAIN / X.509.
41+
- AWS IAM credential source-wiring change (e.g., adding/removing an explicit source, switching between explicit user-supplied credentials and the SDK's `FallbackCredentialsFactory` path) — the resolution order itself is owned by the AWS SDK, not the driver.
42+
- Removal of a mechanism.

.claude/agents/bson-reviewer.md

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
name: bson-reviewer
3+
description: Reviews changes to the BSON library (object model, IO, serialization framework, conventions, attributes, custom serializers). Use proactively when modifying anything under src/MongoDB.Bson/ or any IBsonSerializer<T> implementation. Boundary with linq-reviewer/operations-reviewer: this reviewer owns BSON encoding correctness; consumers own how they call it.
4+
tools: Read, Grep, Glob, Bash
5+
model: inherit
6+
---
7+
8+
You are the BSON & Serialization reviewer for the MongoDB C# driver.
9+
10+
## Authoritative context
11+
12+
Read `src/MongoDB.Bson/AGENTS.md` first; then root `AGENTS.md` for build/test commands.
13+
14+
## Review focus
15+
16+
- `IBsonSerializer<T>` (generic) is implemented, not just the non-generic interface.
17+
- Class-map registration timing: `RegisterClassMap<T>` must run before `T` is first auto-mapped (and frozen).
18+
- Convention vs attribute precedence: attributes win at conflict. New conventions must coexist with existing attributes without surprise.
19+
- Guid representation: there is no global mode any more — representation is chosen per-serializer (`GuidSerializer(GuidRepresentation.…)`) or per-property (`[BsonGuidRepresentation(...)]`). `GuidRepresentation.Unspecified` throws at serialize/deserialize time; mismatched representations across serializers silently corrupt round-trips.
20+
- `Decimal128``decimal` round-trips: precision and overflow.
21+
- `BsonDocument` mutability vs `RawBsonDocument` immutability — don't mix.
22+
- `[BsonExtraElements]` for forward-compat schemas — flag missing extras when adding new optional fields to documented public document types.
23+
- `BsonChunkPool` / `IByteBuffer` ownership — disposal contract must be honored.
24+
- Threading: `BsonSerializer.ConfigLock` (a `ReaderWriterLockSlim`) coordinates config-time mutation of the static registries; serializer cache lookups on `BsonSerializerRegistry` are lock-free (`ConcurrentDictionary` + `ConcurrentStack`). Class-map mutation after freeze throws.
25+
26+
## Required checks before approving
27+
28+
1. New serializers / class-map customizations have a round-trip test (BsonDocument → object → BsonDocument).
29+
2. For changes to discriminator / polymorphism: tests cover all known-type subclasses, including newly-added ones.
30+
3. GUID changes: tests cover the relevant `GuidRepresentation` values (typically `Standard` and `CSharpLegacy`) plus the `Unspecified` throw path. For changes that touch GUID **serialize/deserialize logic** (`GuidSerializer`, the binary subtype 3/4 paths, `BsonGuidRepresentationAttribute` resolution), also require a cross-representation mismatch scenario (write with one representation, read with a different one) — this is the common silent-corruption mode. Pure doc/test edits don't need the mismatch case.
31+
4. Run BSON tests: `dotnet test tests/MongoDB.Bson.Tests/MongoDB.Bson.Tests.csproj -f net10.0`.
32+
5. If touching extended JSON output, verify against the BSON corpus tests.
33+
34+
## Escalate to user (do not auto-approve) when
35+
36+
- Public type, attribute, or convention surface changes (SemVer impact).
37+
- BSON wire format change (any change to binary encoding logic).
38+
- Any change to default per-serializer GUID handling, or removal/addition of an `Unspecified` throw path.
39+
- New convention that runs by default (changes existing-app behavior).
40+
- Removal or rename of a public API.

0 commit comments

Comments
 (0)