Skip to content

Commit dd61ff5

Browse files
committed
Apply /review-areas -all feedback to AGENTS.md docs
1 parent 6d15aec commit dd61ff5

22 files changed

Lines changed: 53 additions & 32 deletions

File tree

.claude/agents/aggregation-reviewer.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,5 @@ Read `src/MongoDB.Driver/AGENTS.md` (router; aggregation/change-stream sections)
3434
- Default value change on `AggregateOptions` / `ChangeStreamOptions`.
3535
- Stage rendering BSON shape change (silent behavior change for users).
3636
- 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).
3738
- Coordination needed with linq-reviewer for new LINQ translators.

.claude/agents/async-reviewer.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ Read the root `AGENTS.md`. The driver maintains paired sync and async public sur
2828

2929
## Required checks before approving
3030

31-
1. Grep the diff for `.Result`, `.Wait()`, `GetAwaiter().GetResult()`, `async void`, `Task.Run`.
31+
1. Grep the diff for `.Result`, `.Wait()`, `GetAwaiter().GetResult()`, `Thread.Sleep`, `async void`, `Task.Run`.
3232
2. Confirm any new `async` library method either takes a `CancellationToken` or has a documented reason not to.
3333
3. For new public async methods, confirm a sync counterpart exists or is intentionally omitted.
3434

@@ -38,3 +38,4 @@ Read the root `AGENTS.md`. The driver maintains paired sync and async public sur
3838
- New public async method without `CancellationToken` and no documented reason.
3939
- New paired sync/async surface that breaks the existing pairing convention.
4040
- 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/builders-reviewer.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ Read `src/MongoDB.Driver/AGENTS.md` (router) first; then root `AGENTS.md` for bu
1313

1414
## Review focus
1515

16-
- `Render` correctness — every `*Definition<T>.Render(RenderArgs<T>)` must produce the documented BSON shape. Note that most types render to `BsonDocument`, but `UpdateDefinition<T>.Render` returns `BsonValue` (pipeline updates render as `BsonArray`) and `ProjectionDefinition<TSource,TProjection>.Render` returns `RenderedProjectionDefinition<TProjection>` — a wrapper of `BsonDocument` plus the projection-result serializer; assert against `.Document`, not the return value directly. Cast/inspect accordingly when asserting. Tests **must** assert rendered output, not just round-trip.
16+
- `Render` correctness — every `*Definition<T>.Render(RenderArgs<T>)` must produce the documented BSON shape. Note that most types render to `BsonDocument`, but `UpdateDefinition<T>.Render` returns `BsonValue` (pipeline updates render as `BsonArray`); `ProjectionDefinition<TSource,TProjection>.Render` returns `RenderedProjectionDefinition<TProjection>` — a wrapper of `BsonDocument` plus the projection-result serializer; and `PipelineDefinition<TInput,TOutput>.Render` returns `RenderedPipelineDefinition<TOutput>` — a wrapper of `IReadOnlyList<BsonDocument>` plus the output serializer. Assert against the wrapped document/list, not the wrapper return value directly. Cast/inspect accordingly when asserting. Tests **must** assert rendered output, not just round-trip.
1717
- `RenderArgs<TDocument>` — passes the document serializer; using the wrong serializer silently emits wrong BSON.
1818
- Lambda overload ambiguity — `Expression<Func<T, …>>` overloads are easy to misroute. Type-parameter explicitness in tests reveals this.
1919
- Implicit conversions — most definition types accept `BsonDocument` and `string` (parsed as JSON — a frequent overload-ambiguity hazard). `Expression<Func<T,…>>` is implicit-convertible specifically on `FilterDefinition<T>`; other definitions accept LINQ expressions only via builder methods. Several non-obvious conversions also exist (`UpdateDefinition<T>``PipelineDefinition<T,T>`; `ProjectionDefinition<TSource,TProjection>``ProjectionDefinition<TSource>`; `FieldDefinition<TDocument>``FieldDefinition<TDocument,TField>`; `PipelineDefinition<TInput,TOutput>` ← four list/array shapes) — see the implicit-conversion bullet list in `src/MongoDB.Driver/AGENTS.md` for the full table. Adding any new implicit conversion risks ambiguity at call sites.
@@ -24,7 +24,7 @@ Read `src/MongoDB.Driver/AGENTS.md` (router) first; then root `AGENTS.md` for bu
2424
## Required checks before approving
2525

2626
1. Render-based unit tests for each new builder method, comparing against expected BSON literals.
27-
2. `dotnet test tests/MongoDB.Driver.Tests/MongoDB.Driver.Tests.csproj -f net10.0 --filter "FullyQualifiedName~FilterDefinition|FullyQualifiedName~UpdateDefinition|FullyQualifiedName~ProjectionDefinition|FullyQualifiedName~SortDefinition|FullyQualifiedName~IndexKeysDefinition|FullyQualifiedName~PipelineDefinition|FullyQualifiedName~ArrayFilterDefinition|FullyQualifiedName~SetFieldDefinitions"`.
27+
2. `dotnet test tests/MongoDB.Driver.Tests/MongoDB.Driver.Tests.csproj -f net10.0 --filter "FullyQualifiedName~FilterDefinition|FullyQualifiedName~UpdateDefinition|FullyQualifiedName~ProjectionDefinition|FullyQualifiedName~SortDefinition|FullyQualifiedName~IndexKeysDefinition|FullyQualifiedName~PipelineDefinition|FullyQualifiedName~ArrayFilterDefinition|FullyQualifiedName~SetFieldDefinitions|FullyQualifiedName~FieldDefinition"`.
2828
3. If lambda-overload changes are involved, build the test project and inspect for new compiler warnings or ambiguity.
2929

3030
## Escalate to user (do not auto-approve) when

.claude/agents/client-api-reviewer.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ Read `src/MongoDB.Driver/AGENTS.md` (router) first; then root `AGENTS.md` for bu
2424

2525
## Required checks before approving
2626

27-
1. Public-API surface diff for the interfaces / settings types — `git diff main -- src/MongoDB.Driver/IMongoClient.cs src/MongoDB.Driver/IMongoDatabase.cs src/MongoDB.Driver/IMongoCollection.cs src/MongoDB.Driver/MongoClient.cs src/MongoDB.Driver/MongoDatabase.cs src/MongoDB.Driver/MongoCollectionImpl*.cs src/MongoDB.Driver/MongoClientSettings.cs src/MongoDB.Driver/MongoUrl*.cs src/MongoDB.Driver/MongoCredential.cs src/MongoDB.Driver/Core/ServerApi*.cs` — read every change. `MongoClient` itself is `public sealed` and directly instantiated by users, so its constructors and public members are part of the SemVer surface. `MongoDatabase.cs` and `MongoCollectionImpl*.cs` are `internal` and are not themselves part of the SemVer surface — the public contract for them is the interfaces above — but you still diff them for behavior / sync-async parity.
27+
1. Public-API surface diff for the interfaces / settings types — `git diff main -- src/MongoDB.Driver/IMongoClient.cs src/MongoDB.Driver/IMongoDatabase.cs src/MongoDB.Driver/IMongoCollection.cs src/MongoDB.Driver/MongoClient.cs src/MongoDB.Driver/MongoDatabase.cs src/MongoDB.Driver/MongoCollectionImpl.cs src/MongoDB.Driver/MongoClientSettings.cs src/MongoDB.Driver/MongoUrl.cs src/MongoDB.Driver/MongoUrlBuilder.cs src/MongoDB.Driver/MongoCredential.cs src/MongoDB.Driver/Core/ServerApi.cs` — read every change. (Globs like `MongoUrl*.cs` don't expand portably in `git diff` arg lists across shells, so the explicit filenames are listed.) `MongoClient` itself is `public sealed` and directly instantiated by users, so its constructors and public members are part of the SemVer surface. `MongoDatabase.cs` and `MongoCollectionImpl.cs` are `internal` and are not themselves part of the SemVer surface — the public contract for them is the interfaces above — but you still diff them for behavior / sync-async parity.
2828
2. Run client/db/collection tests: `dotnet test tests/MongoDB.Driver.Tests/MongoDB.Driver.Tests.csproj -f net10.0 --filter "FullyQualifiedName~MongoClient|FullyQualifiedName~MongoDatabase|FullyQualifiedName~MongoCollection"`.
2929
3. Settings tests: `--filter "FullyQualifiedName~MongoClientSettings|FullyQualifiedName~MongoUrl"`.
3030

.claude/agents/gridfs-reviewer.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ Read `src/MongoDB.Driver/GridFS/AGENTS.md` first; then root `AGENTS.md` for buil
2525
## Required checks before approving
2626

2727
1. `dotnet test tests/MongoDB.Driver.Tests/MongoDB.Driver.Tests.csproj -f net10.0 --filter "FullyQualifiedName~GridFS"`.
28-
2. The JSON-driven GridFS operations under `tests/MongoDB.Driver.Tests/JsonDrivenTests/` and `tests/MongoDB.Driver.Tests/UnifiedTestOperations/` — these exercise transactions / retryable-read coverage for GridFS that the GridFS xUnit suite doesn't.
28+
2. The JSON-driven GridFS operations under `tests/MongoDB.Driver.Tests/JsonDrivenTests/` and `tests/MongoDB.Driver.Tests/UnifiedTestOperations/` — these exercise transactions / retryable-read coverage for GridFS that the GridFS xUnit suite doesn't. When stream lifecycle or transaction behavior is in flight, a focused second pass with `--filter "FullyQualifiedName~JsonDrivenGridFs|FullyQualifiedName~UnifiedGridFs"` runs just those dispatchers.
2929
3. For stream-related changes, dispose-path tests cover both happy and exception cases.
3030

3131
## Escalate to user (do not auto-approve) when

.claude/agents/linq-reviewer.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,5 +36,5 @@ Read `src/MongoDB.Driver/Linq/AGENTS.md` first; then root `AGENTS.md` for build/
3636
- A method's BSON output changes (silent behavior change for users).
3737
- New `ExpressionTranslationOptions` flag with non-default behavior.
3838
- Removal of LINQ method support.
39-
- Major refactor across `Translators/`, `Ast/`, or `SerializerFinders/`.
39+
- Major refactor across `Translators/`, `Ast/`, `SerializerFinders/`, or `Misc/PartialEvaluator` (closure-capture changes have wide blast radius).
4040
- Behavior change in client-side projection fallback.

.claude/agents/search-reviewer.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ Read `src/MongoDB.Driver/Search/AGENTS.md` first; then root `AGENTS.md` for buil
1616
- `$search` / `$vectorSearch` must be the first stage — server-enforced; we don't validate, but we shouldn't generate later-stage variants.
1717
- Compound clauses (`must`, `mustNot`, `should`, `filter`) — semantics on score and inclusion are subtly different; preserve them.
1818
- Vector search tuning — `NumberOfCandidates` >> `Limit` for ANN; ignored when `Exact = true`.
19-
- `QueryVector` types — most input shapes have both an explicit constructor and an implicit conversion: `double[]` / `float[]` / `int[]` / `ReadOnlyMemory<double|float|int>` (raw numeric embeddings), `BinaryVectorInt8` / `BinaryVectorFloat32` / `BinaryVectorPackedBit` (typed BinaryVector encodings), and `string` (auto-embedding flow only). The one exception is `BsonBinaryData`, which is **constructor-only** — there is no implicit conversion. There are no `.Embedded(...)` / `.BinaryVector(...)` factory methods.
19+
- `QueryVector` types — the public constructor set is narrower than the implicit-conversion set, so don't assume one mirrors the other. Public ctors: `string` (auto-embedding), `BsonBinaryData` (the one input shape with **no** implicit conversion), and `ReadOnlyMemory<double|float|int>`. Implicit conversions additionally cover `double[]` / `float[]` / `int[]` and the three BinaryVector types (`BinaryVectorInt8` / `BinaryVectorFloat32` / `BinaryVectorPackedBit`) — `new QueryVector(myDoubleArray)` does **not** compile. There are no `.Embedded(...)` / `.BinaryVector(...)` factory methods.
2020
- `UseConfiguredSerializers` — extension method on `SearchDefinition<T>` that downcasts to `OperatorSearchDefinition<T>` (throws `NotSupportedException` otherwise); in practice meaningful on value-comparing operators (Equals/In/Range). **Default is `true`** (registered serializers honored, e.g. enum-as-string). Affects custom-enum representation; flipping the default to `false` is a breaking change.
2121
- Index helpers — `CreateSearchIndexModel`, plus the vector-search family rooted at the abstract `CreateVectorSearchIndexModelBase<TDocument>` with concrete `CreateVectorSearchIndexModel<TDocument>` and `CreateAutoEmbeddingVectorSearchIndexModel<TDocument>`. Coordinate with builders-reviewer.
2222
- All tests gated by `ATLAS_SEARCH_TESTS_ENABLED` and `ATLAS_SEARCH_URI`. Index-helper tests additionally need `ATLAS_SEARCH_INDEX_HELPERS_TESTS_ENABLED`.

src/MongoDB.Driver.Authentication.AWS/AGENTS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,6 @@ For full coverage — credential resolution (explicit MongoCredential vs the AWS
1313

1414
Wiring entry point: consumers call `MongoClientSettings.Extensions.AddAWSAuthentication()` once to opt into this assembly. That call resolves to the `ExtensionManagerExtensions.AddAWSAuthentication` extension method on `IExtensionManager` (in `src/MongoDB.Driver.Authentication.AWS/ExtensionManagerExtensions.cs`), which registers both `MONGODB-AWS` with `SaslMechanisms` and the AWS KMS provider with `KmsProviders`. The two are registered together because they share the same `aws-sdk-net` dependency this assembly carries — having opted in to the AWS SDK at all, registering the KMS provider too is essentially free and avoids a second opt-in for CSFLE consumers; auth-only consumers can leave `KmsProviders` empty and the KMS-side registration is inert. Nothing in this project registers itself at assembly load; the call is explicit.
1515

16+
For CSFLE-side KMS configuration and the broader Client-Side Encryption pipeline, see `src/MongoDB.Driver.Encryption/AGENTS.md`.
17+
1618
Reviewer: `auth-reviewer`.

src/MongoDB.Driver.Encryption/AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ This project wraps **libmongocrypt** (the C library that implements CSFLE and Qu
3939

4040
libmongocrypt asks managed code for AES / HMAC / random / RSA signing via callbacks. The bundled libmongocrypt build the .NET driver ships routes crypto through these managed callbacks rather than depending on OpenSSL on the host — relevant when reading libmongocrypt upstream docs that describe a default OpenSSL link:
4141

42-
- `CipherCallbacks` — AES-CBC and AES-ECB (in `CipherMode.CBC` / `CipherMode.ECB`) via `System.Security.Cryptography.Aes`. Both modes are required by the libmongocrypt callback contract regardless of what the host's platform crypto offers, because libmongocrypt asks for ECB and CBC primitives separately and composes them into the higher-level key-derivation / key-wrap constructionsECB is **not** used to encrypt plaintext records. See the upstream libmongocrypt sources for the exact constructions.
42+
- `CipherCallbacks` — AES-CBC and AES-ECB (in `CipherMode.CBC` / `CipherMode.ECB`) via `System.Security.Cryptography.Aes`. Both modes are required by the libmongocrypt callback contract regardless of what the host's platform crypto offers, because libmongocrypt asks for ECB and CBC primitives separately and composes them into the higher-level key-derivation / key-wrap constructions. ECB is only used by libmongocrypt internally for key wrap / derivation primitives — never to encrypt user data records. See the upstream libmongocrypt sources for the exact constructions.
4343
- `HmacShaCallbacks` — HMAC-SHA-256 / HMAC-SHA-512.
4444
- `SecureRandomCallback``RandomNumberGenerator.GetBytes`.
4545
- `SigningRSAESPKCSCallback` — RSASSA-PKCS1-v1_5 with SHA-256 (`RSACryptoServiceProvider.SignData`), used by KMIP and Azure key wrapping. The name contains "PKCS" which correctly reflects the signing scheme (not RSA-OAEP, which is an encryption/key-wrap scheme). The PKCS#8 private-key import path requires .NET Core / .NET 5+; on .NET Framework the callback throws `PlatformNotSupportedException`.

src/MongoDB.Driver/AGENTS.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ The same `*Definition` / `*DefinitionBuilder` pattern is used by `FilterDefiniti
6060
| `ArrayFilterDefinition<TItem>` || ✅ (JSON) ||
6161
| `FieldDefinition<T>` || ✅ (treated as a **literal field name** via `StringFieldDefinition<T>`*not* JSON-parsed; this is the lone asymmetry vs. the other definitions' string conversions) ||
6262
| `FieldDefinition<TDocument, TField>` || ✅ (also a literal field name) | up-cast **to** `FieldDefinition<TDocument>` (drops field-CLR-type parameter) |
63-
| `PipelineDefinition<TInput, TOutput>` ||| `IPipelineStageDefinition[]`, `List<IPipelineStageDefinition>`, `BsonDocument[]`, `List<BsonDocument>` (arrays/lists of `BsonDocument`, **not** a singular `BsonDocument`) |
64-
| `SetFieldDefinitions<T>` ||||
63+
| `PipelineDefinition<TInput, TOutput>` ||| `IPipelineStageDefinition[]`, `List<IPipelineStageDefinition>`, `BsonDocument[]`, `List<BsonDocument>` (arrays/lists of `BsonDocument`, **not** a singular `BsonDocument` — wrap a single doc in `new[] { doc }` to use the array conversion) |
64+
| `SetFieldDefinitions<T>` (plural, the collection type exposed via `Builders<T>.SetFields`; distinct from the singular `SetFieldDefinition<T>`, no `s`) ||||
6565

6666
The four `PipelineDefinition<TInput, TOutput>` array/list conversions are a known overload-resolution hazard: a method overloaded on both `PipelineDefinition<…>` and one of those collection shapes may resolve to either depending on the call site. `Expression<Func<T, bool>>` is implicit-convertible **only** on `FilterDefinition<T>`; other definitions accept LINQ expressions exclusively via builder methods.
6767

@@ -72,7 +72,7 @@ The same `*Definition` / `*DefinitionBuilder` pattern is used by `FilterDefiniti
7272
## Aggregation fluent API
7373

7474
- `IAggregateFluent<TResult>` / `AggregateFluentBase<TResult>` (public abstract) / `AggregateFluent<TInput,TResult>` (`internal abstract`) — fluent stage chaining over a `PipelineDefinition`.
75-
- Stage options and result records: `AggregateOptions` (consult `AggregateOptions.cs` for the full list of properties; load-bearing ones include `AllowDiskUse`, `BatchSize`, `MaxAwaitTime`, `Collation`, `Let`, `BypassDocumentValidation`, and `TranslationOptions`; **`UseCursor` is `[Obsolete]`** and new code should not propagate it to any new API surface), `AggregateBucket*`, `AggregateFacet*`, `AggregateGraphLookupOptions`, `AggregateLookupOptions`, `AggregateUnwindOptions`. `AggregateHelper` is `internal static` (a render helper), not part of the public surface. `MaxAwaitTime` (here on `AggregateOptions`, mirrored on `ChangeStreamOptions`) is honored only by **tailable / change-stream** cursors (it bounds the server-side wait on `getMore`); it is **inert** for ordinary aggregation cursors. Don't set it expecting it to bound a regular aggregation.
75+
- Stage options and result records: `AggregateOptions` (consult `AggregateOptions.cs` for the full list of properties; load-bearing ones include `AllowDiskUse`, `BatchSize`, `Comment`, `Hint`, `MaxTime`, `MaxAwaitTime`, `Collation`, `Let`, `BypassDocumentValidation`, and `TranslationOptions`; **`UseCursor` is `[Obsolete]`** and new code should not propagate it to any new API surface), `AggregateBucket*`, `AggregateFacet*`, `AggregateGraphLookupOptions`, `AggregateLookupOptions`, `AggregateUnwindOptions`. `AggregateHelper` is `internal static` (a render helper), not part of the public surface. `MaxAwaitTime` (here on `AggregateOptions`, mirrored on `ChangeStreamOptions`) is honored only by **tailable / change-stream** cursors (it bounds the server-side wait on `getMore`); it is **inert** for ordinary aggregation cursors — see `AggregateOptions.cs` for the property definition and surrounding context. Don't set it expecting it to bound a regular aggregation.
7676
- `AggregateExpressionDefinition<TSource, TResult>` is separate — an aggregation-expression abstraction with concrete subclasses `BsonValueAggregateExpressionDefinition<TSource, TResult>`, `ExpressionAggregateExpressionDefinition<TSource, TResult>`, and `DocumentsAggregateExpressionDefinition<TDocument>`. The last is the input shape for `$documents` and is accepted only by the client- and database-level entry points `IMongoClient.Aggregate(...)` / `IMongoDatabase.Aggregate(...)`; there is no collection-level fluent equivalent (collection-level aggregates run against the collection itself). Used by stage builders that take expression arguments, not itself a stage option.
7777
- Bottoms out in `Core/Operations/AggregateOperation<T>` and `AggregateToCollectionOperation` (for `$out` / `$merge`).
7878

0 commit comments

Comments
 (0)