CSHARP-5943: Add per-area AGENTS.md files and reviewer sub-agents#1993
Open
ajcvickers wants to merge 35 commits into
Open
CSHARP-5943: Add per-area AGENTS.md files and reviewer sub-agents#1993ajcvickers wants to merge 35 commits into
ajcvickers wants to merge 35 commits into
Conversation
ajcvickers
added
the
maintenance
Contributor
Author
|
Here's example output from Summary17 reviewers ran (14 area + 3 cross-cutting). 14 approved, 3 flagged, 0 escalated. EscalationsNone. Cross-cutting findings
Area findingsFlagged spec-conformance-reviewer — flag
Tests run: gridfs-reviewer — flag
geojson-reviewer — flag
Approved (with notes) bson-reviewer — approve
operations-reviewer — approve
auth-reviewer — approve
linq-reviewer — approve
encryption-reviewer — approve
diagnostics-reviewer — approve
transport-reviewer, client-api-reviewer, builders-reviewer, aggregation-reviewer, search-reviewer — all clean. Unmapped changes These files weren't matched by any area reviewer's path pattern (cross-cutters still covered them):
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a structured “functional area” documentation and review-agent layout across the repo to help agent-aware tooling load the right context per directory and to enable parallel, area-scoped reviews.
Changes:
- Added per-area
AGENTS.mdfiles (with YAML frontmatter) and sibling one-lineCLAUDE.md@AGENTS.mdpointers acrosssrc/andtests/. - Added read-only reviewer sub-agents under
.claude/agents/plus a/review-areascommand to fan out reviews by path ownership. - Updated root
AGENTS.md, addeddocs/agents-architecture.md, and adjusted.gitignorefor local Claude files.
Reviewed changes
Copilot reviewed 54 out of 55 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/MongoDB.Driver.Tests/Specifications/CLAUDE.md | Adds local @AGENTS.md pointer for spec runner area. |
| tests/MongoDB.Driver.Tests/Specifications/AGENTS.md | Documents spec conformance runner layout, pitfalls, and test commands. |
| tests/MongoDB.Driver.TestHelpers/CLAUDE.md | Adds local @AGENTS.md pointer for driver test helpers area. |
| tests/MongoDB.Driver.TestHelpers/AGENTS.md | Documents shared driver test infrastructure and conventions. |
| src/MongoDB.Driver/Search/CLAUDE.md | Adds local @AGENTS.md pointer for Search area. |
| src/MongoDB.Driver/Search/AGENTS.md | Documents Atlas Search / Vector Search area and testing gates. |
| src/MongoDB.Driver/Linq/CLAUDE.md | Adds local @AGENTS.md pointer for LINQ area. |
| src/MongoDB.Driver/Linq/AGENTS.md | Documents Linq3 architecture, translators, and testing guidance. |
| src/MongoDB.Driver/GridFS/CLAUDE.md | Adds local @AGENTS.md pointer for GridFS area. |
| src/MongoDB.Driver/GridFS/AGENTS.md | Documents GridFS surface, internals, and test filter. |
| src/MongoDB.Driver/GeoJsonObjectModel/CLAUDE.md | Adds local @AGENTS.md pointer for GeoJSON area. |
| src/MongoDB.Driver/GeoJsonObjectModel/AGENTS.md | Documents GeoJSON model types/serializers and geospatial pitfalls. |
| src/MongoDB.Driver/Encryption/CLAUDE.md | Adds local @AGENTS.md pointer for driver-side encryption glue. |
| src/MongoDB.Driver/Encryption/AGENTS.md | Documents driver-side encryption integration points and testing. |
| src/MongoDB.Driver/Core/Operations/CLAUDE.md | Adds local @AGENTS.md pointer for Operations area. |
| src/MongoDB.Driver/Core/Operations/AGENTS.md | Documents operations/bindings/sessions/transactions responsibilities and tests. |
| src/MongoDB.Driver/Core/Logging/CLAUDE.md | Adds local @AGENTS.md pointer for Logging area. |
| src/MongoDB.Driver/Core/Logging/AGENTS.md | Documents structured logging layer and test filters. |
| src/MongoDB.Driver/Core/Events/CLAUDE.md | Adds local @AGENTS.md pointer for Events area. |
| src/MongoDB.Driver/Core/Events/AGENTS.md | Documents event subsystem design constraints and tests. |
| src/MongoDB.Driver/Core/CLAUDE.md | Adds local @AGENTS.md pointer for Core transport area. |
| src/MongoDB.Driver/Core/Bindings/CLAUDE.md | Adds local @AGENTS.md pointer for Bindings cross-ref area. |
| src/MongoDB.Driver/Core/Bindings/AGENTS.md | Cross-references ownership to Operations area. |
| src/MongoDB.Driver/Core/AGENTS.md | Documents SDAM/transport area architecture, pitfalls, and test filters. |
| src/MongoDB.Driver/CLAUDE.md | Adds local @AGENTS.md pointer for driver root router area. |
| src/MongoDB.Driver/Authentication/CLAUDE.md | Adds local @AGENTS.md pointer for Authentication area. |
| src/MongoDB.Driver/Authentication/AGENTS.md | Documents auth mechanisms, speculative auth, and tests/env vars. |
| src/MongoDB.Driver/AGENTS.md | Adds driver-root “router” AGENTS.md guiding contributors to the right sub-area docs. |
| src/MongoDB.Driver.Encryption/CLAUDE.md | Adds local @AGENTS.md pointer for libmongocrypt wrapper project. |
| src/MongoDB.Driver.Encryption/AGENTS.md | Documents libmongocrypt wrapper architecture, pitfalls, and test env vars. |
| src/MongoDB.Driver.Authentication.AWS/CLAUDE.md | Adds local @AGENTS.md pointer for optional AWS auth project. |
| src/MongoDB.Driver.Authentication.AWS/AGENTS.md | Cross-references AWS IAM auth ownership to Authentication area. |
| src/MongoDB.Bson/CLAUDE.md | Adds local @AGENTS.md pointer for BSON area. |
| src/MongoDB.Bson/AGENTS.md | Documents BSON object model/serialization framework and testing. |
| docs/agents-architecture.md | Describes the overall per-area AGENTS + reviewer-agent architecture and conventions. |
| AGENTS.md | Adds functional area index and lists cross-cutting reviewers. |
| .gitignore | Ignores CLAUDE.local.md. |
| .claude/commands/review-areas.md | Adds /review-areas command to dispatch area reviewers and aggregate results. |
| .claude/agents/transport-reviewer.md | Defines read-only transport/SDAM reviewer agent. |
| .claude/agents/spec-conformance-reviewer.md | Defines read-only spec conformance/test infra reviewer agent. |
| .claude/agents/security-reviewer.md | Defines read-only cross-cutting security reviewer agent. |
| .claude/agents/search-reviewer.md | Defines read-only Search/Vector Search reviewer agent. |
| .claude/agents/operations-reviewer.md | Defines read-only operations/sessions/transactions reviewer agent. |
| .claude/agents/linq-reviewer.md | Defines read-only LINQ reviewer agent. |
| .claude/agents/gridfs-reviewer.md | Defines read-only GridFS reviewer agent. |
| .claude/agents/geojson-reviewer.md | Defines read-only GeoJSON reviewer agent. |
| .claude/agents/encryption-reviewer.md | Defines read-only CSFLE/QE reviewer agent. |
| .claude/agents/diagnostics-reviewer.md | Defines read-only events/logging reviewer agent. |
| .claude/agents/client-api-reviewer.md | Defines read-only public client API reviewer agent. |
| .claude/agents/builders-reviewer.md | Defines read-only CRUD builders DSL reviewer agent. |
| .claude/agents/bson-reviewer.md | Defines read-only BSON/serialization reviewer agent. |
| .claude/agents/auth-reviewer.md | Defines read-only authentication reviewer agent. |
| .claude/agents/async-reviewer.md | Defines read-only cross-cutting async/threading hygiene reviewer agent. |
| .claude/agents/api-stability-reviewer.md | Defines read-only cross-cutting SemVer/public API stability reviewer agent. |
| .claude/agents/aggregation-reviewer.md | Defines read-only aggregation/change streams reviewer agent. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
I've been getting these reviewers to iteratively review their own instructions. I then asked Claude to analyze how this is going: Analysis of /review-areas Iteration QualityI read the diffs of all nine review-feedback commits on this branch and walked the per-file history of the ten files modified in 6+ rounds. Here's what I found. Quality trajectory:
Verdict
Flip-flops: four genuine onesFour specific claims oscillated rather than progressed monotonically:
Two of the four reversals had iter-commit messages that explicitly acknowledged undoing the earlier round — honest self-correction of bad assertions, not bikeshedding. Hot spots: mostly legitimate complexity, one weak spotOf the ten files modified in 6+ rounds, eight reappear because the underlying subsystem is genuinely intricate and each pass surfaced new real errors — BSON, Driver root, LINQ (Linq3 has its own AST), Authentication (six mechanisms × SASL state), Core/SDAM, Core/Operations (retry executors, bindings, transactions), Encryption (libmongocrypt P/Invoke + KMS providers), and GridFS. One file is a clear casualty of polish-for-its-own-sake: src/MongoDB.Driver/Core/Logging/AGENTS.md. The MaxDocumentSize=0 wording was tightened in four separate rounds (iters 1, 3, 6, 9) with little semantic change after iter 3. Events is borderline. Bottom lineQuality improved meaningfully through ~iter 7. Returns are clearly diminishing (per-iteration diff size has dropped ~3×). Four specific claims flip-flopped — concentrated in 3–4 files, not pervasive. If you wanted to stop the experiment, iter 8 or 9 was probably the right cutoff; one more round would mostly produce rewording, except possibly on Authentication and BSON where the subsystems are dense enough that surprises keep landing. |
Contributor
Author
|
Additional analysis after more runs. Confirms diminishing returns. CSHARP-5943c: Iterative
|
| Phase | Commits | Pattern |
|---|---|---|
| Initial | 71eeb36, eca90f4 | Seed: ~2,600 lines of new AGENTS.md + cross-cutting reviewers |
| High-value corrections | 4995bef → 12bff25 (≈10 passes) | 3–7 substantive fixes per pass. Wrong type names, wrong order-of-operations, missing entry points, inverted rules (e.g. protected internal wider than protected; MinPoolSize → MinConnections; AllowInsecureTls also bypasses hostname matching; bulk-write result/exception generic sealed types). |
| Refinement / nit | 1fc9ce6 → acab029 (≈8 passes) | 1–2 substantive items per pass, surrounded by rephrasing, anchor splits, parenthetical adds. Often re-touches the same area the previous commit just touched (e.g. QueryVector ctor set adjusted twice, GridFS Delete reworded twice). |
Inflection commit is 1fc9ce6 (commit #24 overall, 11th in the late series): 19 files but only ~3 substantive fixes. Reasonable "stop here" candidates: 12bff25 (last clean substantive cluster) or 8e3cb1e (after which only ~2 substantive items land across 4 more commits). Roughly the last ~7 passes are net polish.
Per-commit categorization (later passes)
| # | Commit | Files | Sub | Ref | Nit | Biggest substantive fix |
|---|---|---|---|---|---|---|
| 1 | c0bca55 | 12 | 4 | 8 | 3 | Bulk-write result/exception are the sealed generic BulkWriteResult<T> / MongoBulkWriteException<T> |
| 2 | e9af324 | 4 | 4 | 2 | 0 | Registry resolution rewritten: ConcurrentStack LIFO, BsonObjectModel first not last |
| 3 | 6a42432 | 4 | 3 | 1 | 1 | Speculative auth includes OIDC (not "only SCRAM and X.509"); AWS chain delegated to SDK |
| 4 | bea7ec3 | 4 | 2 | 2 | 1 | Drop bogus "4.4.2+" helloOk version gate; PathPrefix vs PathPrefixes inconsistency |
| 5 | 50ea114 | 10 | 7 | 3 | 1 | protected internal is wider than protected (visibility-tightening rule was inverted); AllowInsecureTls bypasses hostname matching too |
| 6 | e96c23e | 3 | 3 | 1 | 0 | MinPoolSize → MinConnections; MaxIdleTime on ConnectionSettings not ConnectionPoolSettings |
| 7 | 88978e1 | 6 | 3 | 3 | 1 | LB mode runs no SDAM heartbeats (no IServerMonitor on LoadBalancedServer) |
| 8 | f99e399 | 3 | 3 | 1 | 0 | UTF ops MUST be wired into UnifiedTestOperationFactory.CreateOperation (prior text said no such factory) |
| 9 | 37e9b13 | 16 | 6 | 7 | 4 | Drop bogus "Vercel" entry / cite ServerMonitor.IsRunningInFaaS; correct FieldDefinition string-conversion; override InitializeFixture/InitializeTestCase, not BeforeTestCase |
| 10 | 12bff25 | 4 | 2 | 2 | 0 | MongoClient is public sealed — its constructors/members are SemVer surface (prior brief said only internals) |
| 11 | 1fc9ce6 | 19 | 3 | 10 | 6 | PipelineDefinition.Render returns RenderedPipelineDefinition<TOutput>, not BsonDocument; ArrayFilterDefinition.Render has a different signature |
| 12 | 9ba8647 | 12 | 2 | 8 | 2 | [BsonGuidRepresentation] overrides class-level / serializer-level setting; RoundTripTimeMonitor always constructed but only Start()-ed in streaming mode |
| 13 | 1c1de12 | 11 | 1 | 6 | 4 | Mostly precision tweaks; no substantive correction |
| 14 | 8e3cb1e | 15 | 2 | 9 | 4 | CSOT-gating note on AbortTransactionOptions/CommitTransactionOptions internal-ness; DelegateServerSelector added to selector list |
| 15 | 312c9b6 | 20 | 1 | 11 | 8 | Add Thread.Sleep inside an async method to the async-reviewer focus list |
| 16 | 0b4e75c | 8 | 1 | 4 | 3 | QueryVector(double[]) ctor does NOT exist — array shapes are implicit-conversion-only |
| 17 | 263ca95 | 22 | 2 | 12 | 8 | EndTransactionOperation's always-retryable rule is a deliberate spec deviation; QueryVector constructor set re-narrowed |
| 18 | acab029 | 12 | 1 | 8 | 3 | SetFields family is odd-one-out: builder singular, definition plural |
True flip-flops
Only three claim-level reversals; one token-level wobble.
helloOkserver version insrc/MongoDB.Driver/Core/AGENTS.md: no version → "5.0+" (e16668f) → "4.4.2+" (842b243) → version removed entirely (bea7ec3). Two reversals on the same line.IDnsResolversubstitutability insrc/MongoDB.Driver/Core/AGENTS.md: "users can substitute a customIDnsResolver" (71eeb36) → "internal, no public seam, internal test seam only" (e96c23e) → softened back to "production abstraction that doubles as a test seam" (9ba8647).$polygonrecommendation insrc/MongoDB.Driver/GeoJsonObjectModel/AGENTS.md: cited as workaround (71eeb36) → "$polygonis a legacy 2d-index operator and does not apply" (263ca95) → entire parenthetical removed (acab029).GuidRepresentationMode.V2/V3token insrc/MongoDB.Bson/AGENTS.md: token mentioned → dropped (e16668f) → re-mentioned (c0bca55). The underlying claim ("mode was removed") stayed consistent — only the surface token came back.
Things that looked like flip-flops but weren't
MinPoolSize→MinConnections: one-way rename late in the series.AggregateToCollectionOperationretryability: monotone refinement (added "not retryable" → refined to "implementsIRetryableWriteOperation<BsonDocument>but reportsIsOperationRetryable => false").DownloadStreamForwardOnly→Seekable = true: one-way correction of an invented API.- "internal sealed" sweeping claim on operations: corrected once to "mostly internal sealed", not re-asserted.
- Pool warm-up wording (lazy vs maintenance-thread): one-way correction.
Takeaway
The loop was clearly productive for the first ~10 passes and then crossed into prose-polishing with a thin substantive long tail. Three real flip-flops are worth a single corrective sweep before merge — particularly the IDnsResolver substitutability stance, which inverted twice and ended up in a middle position that may still misrepresent that the interface is internal.
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.
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) |
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.
…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.
…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.
…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.
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.
- 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.
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.
…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.
- 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).
- 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).
…tions 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.
- 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.
- 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.
…tions 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.
…tions - 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.
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.
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).
Rename /review-areas to /review-csharp-driver across the skill, its
agent briefs, and the architecture doc.
Three substantive additions:
* External clone mode: pass a directory path and the skill reviews
the branch checked out in that clone while running with the
current clone's reviewer briefs and AGENTS.md. Useful when the
branch under review predates the latest agent updates - clone
twice, run the skill from the up-to-date clone, operate on the
other. Reviewer prompts get absolute paths and `git -C <clone>`
so file reads and git history come from the right tree while
AGENTS.md context loads from the parent's cwd.
* --iterate / --max-iterations (default 10): after each review the
parent dispatches a general-purpose fixer subagent that edits and
commits inside <diff-repo>. Loop stops at two consecutive clean
reviews, on max iterations, on fixer failure, or on a no-op
fixer. Allowed in local-range and external-clone modes; rejected
for external PR mode since we don't push back to PR branches.
* Convergence heuristics:
- Reviewers tag every finding [fix-in-code] or [external-action];
only [fix-in-code] flag findings count toward the clean check,
so JIRA references, CI-matrix verification, "audit callers
outside the diff" etc. surface to the user but don't pin the
loop open.
- A no-op fixer ends the loop with outcome "no actionable
findings remaining" rather than re-rolling reviewer verdicts
on an unchanged tree.
- Outstanding [external-action] items are listed in the closing
summary so the user knows what still needs handling.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.