From 272ca93db571fcbc7fbcb988fabfd3c0cbf914ed Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 25 Sep 2026 14:08:51 -0700 Subject: [PATCH 1/5] docs(review): rule 24 gains the update shape of the strict-subdocument drop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit vera asked (74186) where #1898's strict-subdocument finding belongs. The class is already rule 24's — a `strict` schema silently drops an undeclared path on the write, so a fixture that builds its own document proves the reader can read a field and never that the field can be there — and rule 37 answers a different question (whether a read tells you the key exists, the value is non-empty, or the value can authenticate). Filing a write-side drop under the read-side rule would hide it from the next reviewer grepping for it, so the instance goes where the class already lives. The added instance is the update shape of the same defect, which rule 24 did not carry: `$set { 'config.webhookUrlRef': ref }` on a row the Discord connect route had just created was dropped in silence because `Integration.config` is a strict subdocument that did not declare the path. Same tell as the constructor case and the same instrument closes it — the only test that saw it asserts against real Mongo after the create call and reads the ref back. Docs-only, one file. --- docs/development/review-checklist.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/development/review-checklist.md b/docs/development/review-checklist.md index d9c0b65a9b..8bdf2924e7 100644 --- a/docs/development/review-checklist.md +++ b/docs/development/review-checklist.md @@ -80,7 +80,7 @@ And then note where the fix landed. Someone hit the audience gap, felt it, and b 23. **When a change alters the shape of a mocked value, the sweep must key on the mocked *path*, and it must not be a same-line conjunction.** A route's query chain changing shape (`find()` → `find().sort()`) breaks every suite whose mock returns the old shape, so the reviewer's sweep is "which suites mock this model" — and the obvious spelling of that sweep is the one that excludes the broken suite. Three populations over `backend/__tests__/**`, measured at `584442e9`: `jest.mock(.*AgentInstallation` on one line returns **7 files and omits the failing suite**; the bare symbol returns 96 and includes it; `jest.mock('…/models/AgentRegistry'` returns **74** and reproduces the failure. The reason the first misses is not that the symbol is absent — it is on the next two lines — but that `jest.mock` factories wrap, so a line-oriented conjunction can never see a symbol declared inside a multi-line call. **Both halves of the fix are load-bearing: key on the module path, because the changed export (`AgentInstallation`) is not the module (`models/AgentRegistry`); and drop the conjunction, because the anchor and the symbol are on different lines.** Generalises past mocks: any sweep written as `.*` on one line silently under-reports over wrapped calls, and rule 12 applies at full strength — the under-report renders as an all-green population, which is the answer the reviewer was hoping for. *(Earned: #1336, 2026-08-30 — a gate that passed on mutations plus the PR's own two suites turned main red for 17½ minutes across the whole fleet; the sibling sweep was run and returned a false all-clear before #1341 fixed the mock. Diagnosis by @sprint-review; the same-line/multi-line mechanism measured while writing this entry.)* -24. **A fixture that builds its own document proves the reader can read a field — never that the field can be there.** Tests written against a hand-constructed object exercise the *read* and skip the *write*, and the write is exactly where a strict schema silently drops what the reader expects: Mongoose strips undeclared paths out of a `strict` subdocument, and `findOneAndUpdate` does not run validators — so a seeder can persist an enum-invalid value while the field beside it never lands at all. The result is the worst pairing: a green test, and a reader whose preferred branch has never once fired on real data. The fallback usually hides it, because a fallback is what makes the undefined field survivable and the defect invisible. **The check is two greps and one round-trip:** for each field the code reads cross-layer, confirm the writing schema declares that exact path; then confirm at least one test round-trips a document through the real model (`new Model(…).toObject()`, or a real upsert against memory Mongo) rather than constructing the shape inline. If every test for a read path builds its own object, the path is unproven however many assertions it carries — and mutating the schema will not redden it. *(Earned: #1734/TASK-043 — `routes/registry/install.ts` derived a runtime type from `manifest.runtime.runtimeType`, and `ManifestRuntimeSchema` had no such path while being `strict`, so the field was dropped on every write: `new AgentRegistry({manifest:{runtime:{type:'native',runtimeType:'native'}}}).toObject().manifest.runtime` returns `{type:'native',connection:'mcp'}`, and `schema.path('manifest.runtime.runtimeType')` is `undefined`. The route's own test stayed green because its fixture was a plain object; the seeder's enum-invalid `type:'native'` persisted precisely because `findOneAndUpdate` skips validators; and the fallback had been dead on every row ever written. The fix that closes it is the same shape everywhere: declare the path, route every writer through one builder, and round-trip the fixture through the model so deleting the schema path fails the route test too.)* +24. **A fixture that builds its own document proves the reader can read a field — never that the field can be there.** Tests written against a hand-constructed object exercise the *read* and skip the *write*, and the write is exactly where a strict schema silently drops what the reader expects: Mongoose strips undeclared paths out of a `strict` subdocument, and `findOneAndUpdate` does not run validators — so a seeder can persist an enum-invalid value while the field beside it never lands at all. The result is the worst pairing: a green test, and a reader whose preferred branch has never once fired on real data. The fallback usually hides it, because a fallback is what makes the undefined field survivable and the defect invisible. **The check is two greps and one round-trip:** for each field the code reads cross-layer, confirm the writing schema declares that exact path; then confirm at least one test round-trips a document through the real model (`new Model(…).toObject()`, or a real upsert against memory Mongo) rather than constructing the shape inline. If every test for a read path builds its own object, the path is unproven however many assertions it carries — and mutating the schema will not redden it. *(Earned: #1734/TASK-043 — `routes/registry/install.ts` derived a runtime type from `manifest.runtime.runtimeType`, and `ManifestRuntimeSchema` had no such path while being `strict`, so the field was dropped on every write: `new AgentRegistry({manifest:{runtime:{type:'native',runtimeType:'native'}}}).toObject().manifest.runtime` returns `{type:'native',connection:'mcp'}`, and `schema.path('manifest.runtime.runtimeType')` is `undefined`. The route's own test stayed green because its fixture was a plain object; the seeder's enum-invalid `type:'native'` persisted precisely because `findOneAndUpdate` skips validators; and the fallback had been dead on every row ever written. The fix that closes it is the same shape everywhere: declare the path, route every writer through one builder, and round-trip the fixture through the model so deleting the schema path fails the route test too.)* *(Second instance of the same class, in the UPDATE shape: 2026-09-25, TASK-124/#1898 — the writer was an `updateOne` `$set` rather than a constructor, and `Integration.config` is a `strict` subdocument that did not declare the path, so `$set { 'config.webhookUrlRef': ref }` on a row the Discord connect route had just created was dropped in silence. Same tell, same instrument: the route's own suite stayed green on inline-built fixtures and the only test that saw it asserts against real Mongo after the create call and reads the ref back (`backend/__tests__/unit/routes/integrations.discordTokenCopy.test.js`). The fix is one declaration — `webhookUrlRef: String` in `backend/models/Integration.ts` beside the other Discord config paths — and the reason it is worth grepping for both shapes: the drop is silent in a way a validation error is not, so the reader simply takes its fallback branch and reports the connector as unavailable. Corollary for the writer's side: prefer a declared path plus a real-Mongo assertion over a mocked `updateOne`, which cannot see the drop at all.)* 25. **Enumerate a guard set by its refusal code, not by its predicate.** Guards that refuse the same operation for the same stated reason are frequently implemented by *different mechanisms*, so a grep for the predicate finds only the ones that happen to share it — and a set you believe is closed is precisely what makes a wrong statement about reachability sound. The mechanical form: grep the **refusal code or status value the client branches on** (frontends catch these by string, which is what makes the code the stable identity), read every hit, and only then say whether a path does or does not reach a gate. A shared error code enumerates a gate set; a predicate enumerates a mechanism. *(Earned: 2026-09-18, #1734 — I grepped `isCloudRuntime`, found exactly two call sites, and told a reviewer "the gate lives only in `routes/registry/install.ts`." The refusal code `cloud_agents_not_entitled` has three sites: `registry/install.ts` and `registry/provision.ts` gate via `isCloudRuntime`, while `personas.ts` gates runtime-agnostically on role + `entitlements.cloudAgents` before calling `hirePersona`. So the hire path was gated all along, and the incomplete map had already been used to argue which paths a ruling had to cover.)* From 8aa22c856d008a30ad7a8af4a010d1e4890ecf64 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 25 Sep 2026 14:15:05 -0700 Subject: [PATCH 2/5] docs(review): point rule 24's second instance at the PR that carries its locators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit wren's hold at 272ca93d: rule 24 is the right home and the suite claim holds, but two locators do not, and the PR merges after #1898. Both halves are the same fact — the declaration (`webhookUrlRef: String` in the `config` subdocument) and the witnessing assertion (`integrations.discordTokenCopy.test.js`) exist only on the unmerged #1898: `git grep webhookUrlRef origin/main -- backend frontend docs` returns nothing. So the clause now names #1898 as the carrier of both, states the pre-merge grep as the rev this was earned at (true forever, and it is the state a reader arriving early would find), and says the merge order outright. The witness is described by its mechanism — the create route against a MongoMemoryServer-backed model, read back with `.lean()` — and the census is the measured one: of the five suites that name the ref, the other four mock `Integration` or seed the ref themselves, so none of them writes it through the route. Docs-only, one line. --- docs/development/review-checklist.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/development/review-checklist.md b/docs/development/review-checklist.md index 8bdf2924e7..ccd6c90700 100644 --- a/docs/development/review-checklist.md +++ b/docs/development/review-checklist.md @@ -80,7 +80,7 @@ And then note where the fix landed. Someone hit the audience gap, felt it, and b 23. **When a change alters the shape of a mocked value, the sweep must key on the mocked *path*, and it must not be a same-line conjunction.** A route's query chain changing shape (`find()` → `find().sort()`) breaks every suite whose mock returns the old shape, so the reviewer's sweep is "which suites mock this model" — and the obvious spelling of that sweep is the one that excludes the broken suite. Three populations over `backend/__tests__/**`, measured at `584442e9`: `jest.mock(.*AgentInstallation` on one line returns **7 files and omits the failing suite**; the bare symbol returns 96 and includes it; `jest.mock('…/models/AgentRegistry'` returns **74** and reproduces the failure. The reason the first misses is not that the symbol is absent — it is on the next two lines — but that `jest.mock` factories wrap, so a line-oriented conjunction can never see a symbol declared inside a multi-line call. **Both halves of the fix are load-bearing: key on the module path, because the changed export (`AgentInstallation`) is not the module (`models/AgentRegistry`); and drop the conjunction, because the anchor and the symbol are on different lines.** Generalises past mocks: any sweep written as `.*` on one line silently under-reports over wrapped calls, and rule 12 applies at full strength — the under-report renders as an all-green population, which is the answer the reviewer was hoping for. *(Earned: #1336, 2026-08-30 — a gate that passed on mutations plus the PR's own two suites turned main red for 17½ minutes across the whole fleet; the sibling sweep was run and returned a false all-clear before #1341 fixed the mock. Diagnosis by @sprint-review; the same-line/multi-line mechanism measured while writing this entry.)* -24. **A fixture that builds its own document proves the reader can read a field — never that the field can be there.** Tests written against a hand-constructed object exercise the *read* and skip the *write*, and the write is exactly where a strict schema silently drops what the reader expects: Mongoose strips undeclared paths out of a `strict` subdocument, and `findOneAndUpdate` does not run validators — so a seeder can persist an enum-invalid value while the field beside it never lands at all. The result is the worst pairing: a green test, and a reader whose preferred branch has never once fired on real data. The fallback usually hides it, because a fallback is what makes the undefined field survivable and the defect invisible. **The check is two greps and one round-trip:** for each field the code reads cross-layer, confirm the writing schema declares that exact path; then confirm at least one test round-trips a document through the real model (`new Model(…).toObject()`, or a real upsert against memory Mongo) rather than constructing the shape inline. If every test for a read path builds its own object, the path is unproven however many assertions it carries — and mutating the schema will not redden it. *(Earned: #1734/TASK-043 — `routes/registry/install.ts` derived a runtime type from `manifest.runtime.runtimeType`, and `ManifestRuntimeSchema` had no such path while being `strict`, so the field was dropped on every write: `new AgentRegistry({manifest:{runtime:{type:'native',runtimeType:'native'}}}).toObject().manifest.runtime` returns `{type:'native',connection:'mcp'}`, and `schema.path('manifest.runtime.runtimeType')` is `undefined`. The route's own test stayed green because its fixture was a plain object; the seeder's enum-invalid `type:'native'` persisted precisely because `findOneAndUpdate` skips validators; and the fallback had been dead on every row ever written. The fix that closes it is the same shape everywhere: declare the path, route every writer through one builder, and round-trip the fixture through the model so deleting the schema path fails the route test too.)* *(Second instance of the same class, in the UPDATE shape: 2026-09-25, TASK-124/#1898 — the writer was an `updateOne` `$set` rather than a constructor, and `Integration.config` is a `strict` subdocument that did not declare the path, so `$set { 'config.webhookUrlRef': ref }` on a row the Discord connect route had just created was dropped in silence. Same tell, same instrument: the route's own suite stayed green on inline-built fixtures and the only test that saw it asserts against real Mongo after the create call and reads the ref back (`backend/__tests__/unit/routes/integrations.discordTokenCopy.test.js`). The fix is one declaration — `webhookUrlRef: String` in `backend/models/Integration.ts` beside the other Discord config paths — and the reason it is worth grepping for both shapes: the drop is silent in a way a validation error is not, so the reader simply takes its fallback branch and reports the connector as unavailable. Corollary for the writer's side: prefer a declared path plus a real-Mongo assertion over a mocked `updateOne`, which cannot see the drop at all.)* +24. **A fixture that builds its own document proves the reader can read a field — never that the field can be there.** Tests written against a hand-constructed object exercise the *read* and skip the *write*, and the write is exactly where a strict schema silently drops what the reader expects: Mongoose strips undeclared paths out of a `strict` subdocument, and `findOneAndUpdate` does not run validators — so a seeder can persist an enum-invalid value while the field beside it never lands at all. The result is the worst pairing: a green test, and a reader whose preferred branch has never once fired on real data. The fallback usually hides it, because a fallback is what makes the undefined field survivable and the defect invisible. **The check is two greps and one round-trip:** for each field the code reads cross-layer, confirm the writing schema declares that exact path; then confirm at least one test round-trips a document through the real model (`new Model(…).toObject()`, or a real upsert against memory Mongo) rather than constructing the shape inline. If every test for a read path builds its own object, the path is unproven however many assertions it carries — and mutating the schema will not redden it. *(Earned: #1734/TASK-043 — `routes/registry/install.ts` derived a runtime type from `manifest.runtime.runtimeType`, and `ManifestRuntimeSchema` had no such path while being `strict`, so the field was dropped on every write: `new AgentRegistry({manifest:{runtime:{type:'native',runtimeType:'native'}}}).toObject().manifest.runtime` returns `{type:'native',connection:'mcp'}`, and `schema.path('manifest.runtime.runtimeType')` is `undefined`. The route's own test stayed green because its fixture was a plain object; the seeder's enum-invalid `type:'native'` persisted precisely because `findOneAndUpdate` skips validators; and the fallback had been dead on every row ever written. The fix that closes it is the same shape everywhere: declare the path, route every writer through one builder, and round-trip the fixture through the model so deleting the schema path fails the route test too.)* *(Second instance of the same class, in the UPDATE shape: 2026-09-25, TASK-124/#1898 — the writer was an `updateOne` `$set` rather than a constructor, and `Integration.config` is a `strict` subdocument that did not declare the path, so `$set { 'config.webhookUrlRef': ref }` on a row the Discord connect route had just created was dropped in silence; at the rev this was earned, grepping `webhookUrlRef` across the backend returned nothing at all. Same tell, same instrument: the route's own suite stayed green on inline-built fixtures, and the assertion that saw the ref missing is the one that drives the create route against a `MongoMemoryServer`-backed model and reads the row back with `.lean()` (`backend/__tests__/unit/routes/integrations.discordTokenCopy.test.js`). The fix is one declaration — `webhookUrlRef: String` in the `config` subdocument of `backend/models/Integration.ts` — carried by the same PR as that assertion, which is why this instance merges **after** #1898. Two things worth carrying forward: the drop is silent in a way a validation error is not, so the reader simply takes its fallback branch and reports the connector as unavailable; and of the five suites that name the ref, the other four mock `Integration` or seed the ref themselves, so none of them writes it through the route — a mocked `updateOne` cannot see the drop at all.)* 25. **Enumerate a guard set by its refusal code, not by its predicate.** Guards that refuse the same operation for the same stated reason are frequently implemented by *different mechanisms*, so a grep for the predicate finds only the ones that happen to share it — and a set you believe is closed is precisely what makes a wrong statement about reachability sound. The mechanical form: grep the **refusal code or status value the client branches on** (frontends catch these by string, which is what makes the code the stable identity), read every hit, and only then say whether a path does or does not reach a gate. A shared error code enumerates a gate set; a predicate enumerates a mechanism. *(Earned: 2026-09-18, #1734 — I grepped `isCloudRuntime`, found exactly two call sites, and told a reviewer "the gate lives only in `routes/registry/install.ts`." The refusal code `cloud_agents_not_entitled` has three sites: `registry/install.ts` and `registry/provision.ts` gate via `isCloudRuntime`, while `personas.ts` gates runtime-agnostically on role + `entitlements.cloudAgents` before calling `hirePersona`. So the hire path was gated all along, and the incomplete map had already been used to argue which paths a ruling had to cover.)* From ee4eaaa3d9f274db2142fef5bb831dfe484a1449 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 25 Sep 2026 14:16:39 -0700 Subject: [PATCH 3/5] docs(review): name the Model-vs-raw-collection split, and the right noun for `config` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit wren's two corrections (74203/74204, relayed by connector-ops at 74205) applied, both verified at the premise first: 1. The writer is not `updateOne`. On #1898 the connect route writes with `Integration.findByIdAndUpdate(..., { $set: { 'config.webhookUrlRef': ref } })` (`routes/integrations.ts:474`) and the backfill does the same (`services/discordService.ts:345`). The only `updateOne` is the migration script's, and it runs on `mongoose.connection.collection()` — a raw-collection write, which strict mode never touches. So the clause now teaches the split instead: **a write through the Model is stripped, a raw-collection write is not**, and it says why the obvious grep points the wrong way (grepping `updateOne` finds the one writer that cannot drop). 2. `config` is not a subdocument. Measured from the model at 715e187c rather than restated: `schema.path('config')` is `undefined`, `schema.nested.config` is true, `schema.paths` carries no `config` key, and `schema.get('strict')` is true. The clause now says "nested path of a strict schema" and points at rule 34, which already describes this same `config` that way. The backfill's drop is marked *derived*, not observed (same schema, same `$set`, and both writers landed in one PR, so the declaration fixed both before either merged). #1898 itself is untouched: its model comment repeats "subdocument", and moving a cleared head that is in the enqueue for a noun is not worth it. Docs-only, one line. --- docs/development/review-checklist.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/development/review-checklist.md b/docs/development/review-checklist.md index ccd6c90700..f33cf7ee4a 100644 --- a/docs/development/review-checklist.md +++ b/docs/development/review-checklist.md @@ -80,7 +80,7 @@ And then note where the fix landed. Someone hit the audience gap, felt it, and b 23. **When a change alters the shape of a mocked value, the sweep must key on the mocked *path*, and it must not be a same-line conjunction.** A route's query chain changing shape (`find()` → `find().sort()`) breaks every suite whose mock returns the old shape, so the reviewer's sweep is "which suites mock this model" — and the obvious spelling of that sweep is the one that excludes the broken suite. Three populations over `backend/__tests__/**`, measured at `584442e9`: `jest.mock(.*AgentInstallation` on one line returns **7 files and omits the failing suite**; the bare symbol returns 96 and includes it; `jest.mock('…/models/AgentRegistry'` returns **74** and reproduces the failure. The reason the first misses is not that the symbol is absent — it is on the next two lines — but that `jest.mock` factories wrap, so a line-oriented conjunction can never see a symbol declared inside a multi-line call. **Both halves of the fix are load-bearing: key on the module path, because the changed export (`AgentInstallation`) is not the module (`models/AgentRegistry`); and drop the conjunction, because the anchor and the symbol are on different lines.** Generalises past mocks: any sweep written as `.*` on one line silently under-reports over wrapped calls, and rule 12 applies at full strength — the under-report renders as an all-green population, which is the answer the reviewer was hoping for. *(Earned: #1336, 2026-08-30 — a gate that passed on mutations plus the PR's own two suites turned main red for 17½ minutes across the whole fleet; the sibling sweep was run and returned a false all-clear before #1341 fixed the mock. Diagnosis by @sprint-review; the same-line/multi-line mechanism measured while writing this entry.)* -24. **A fixture that builds its own document proves the reader can read a field — never that the field can be there.** Tests written against a hand-constructed object exercise the *read* and skip the *write*, and the write is exactly where a strict schema silently drops what the reader expects: Mongoose strips undeclared paths out of a `strict` subdocument, and `findOneAndUpdate` does not run validators — so a seeder can persist an enum-invalid value while the field beside it never lands at all. The result is the worst pairing: a green test, and a reader whose preferred branch has never once fired on real data. The fallback usually hides it, because a fallback is what makes the undefined field survivable and the defect invisible. **The check is two greps and one round-trip:** for each field the code reads cross-layer, confirm the writing schema declares that exact path; then confirm at least one test round-trips a document through the real model (`new Model(…).toObject()`, or a real upsert against memory Mongo) rather than constructing the shape inline. If every test for a read path builds its own object, the path is unproven however many assertions it carries — and mutating the schema will not redden it. *(Earned: #1734/TASK-043 — `routes/registry/install.ts` derived a runtime type from `manifest.runtime.runtimeType`, and `ManifestRuntimeSchema` had no such path while being `strict`, so the field was dropped on every write: `new AgentRegistry({manifest:{runtime:{type:'native',runtimeType:'native'}}}).toObject().manifest.runtime` returns `{type:'native',connection:'mcp'}`, and `schema.path('manifest.runtime.runtimeType')` is `undefined`. The route's own test stayed green because its fixture was a plain object; the seeder's enum-invalid `type:'native'` persisted precisely because `findOneAndUpdate` skips validators; and the fallback had been dead on every row ever written. The fix that closes it is the same shape everywhere: declare the path, route every writer through one builder, and round-trip the fixture through the model so deleting the schema path fails the route test too.)* *(Second instance of the same class, in the UPDATE shape: 2026-09-25, TASK-124/#1898 — the writer was an `updateOne` `$set` rather than a constructor, and `Integration.config` is a `strict` subdocument that did not declare the path, so `$set { 'config.webhookUrlRef': ref }` on a row the Discord connect route had just created was dropped in silence; at the rev this was earned, grepping `webhookUrlRef` across the backend returned nothing at all. Same tell, same instrument: the route's own suite stayed green on inline-built fixtures, and the assertion that saw the ref missing is the one that drives the create route against a `MongoMemoryServer`-backed model and reads the row back with `.lean()` (`backend/__tests__/unit/routes/integrations.discordTokenCopy.test.js`). The fix is one declaration — `webhookUrlRef: String` in the `config` subdocument of `backend/models/Integration.ts` — carried by the same PR as that assertion, which is why this instance merges **after** #1898. Two things worth carrying forward: the drop is silent in a way a validation error is not, so the reader simply takes its fallback branch and reports the connector as unavailable; and of the five suites that name the ref, the other four mock `Integration` or seed the ref themselves, so none of them writes it through the route — a mocked `updateOne` cannot see the drop at all.)* +24. **A fixture that builds its own document proves the reader can read a field — never that the field can be there.** Tests written against a hand-constructed object exercise the *read* and skip the *write*, and the write is exactly where a strict schema silently drops what the reader expects: Mongoose strips undeclared paths out of a `strict` subdocument, and `findOneAndUpdate` does not run validators — so a seeder can persist an enum-invalid value while the field beside it never lands at all. The result is the worst pairing: a green test, and a reader whose preferred branch has never once fired on real data. The fallback usually hides it, because a fallback is what makes the undefined field survivable and the defect invisible. **The check is two greps and one round-trip:** for each field the code reads cross-layer, confirm the writing schema declares that exact path; then confirm at least one test round-trips a document through the real model (`new Model(…).toObject()`, or a real upsert against memory Mongo) rather than constructing the shape inline. If every test for a read path builds its own object, the path is unproven however many assertions it carries — and mutating the schema will not redden it. *(Earned: #1734/TASK-043 — `routes/registry/install.ts` derived a runtime type from `manifest.runtime.runtimeType`, and `ManifestRuntimeSchema` had no such path while being `strict`, so the field was dropped on every write: `new AgentRegistry({manifest:{runtime:{type:'native',runtimeType:'native'}}}).toObject().manifest.runtime` returns `{type:'native',connection:'mcp'}`, and `schema.path('manifest.runtime.runtimeType')` is `undefined`. The route's own test stayed green because its fixture was a plain object; the seeder's enum-invalid `type:'native'` persisted precisely because `findOneAndUpdate` skips validators; and the fallback had been dead on every row ever written. The fix that closes it is the same shape everywhere: declare the path, route every writer through one builder, and round-trip the fixture through the model so deleting the schema path fails the route test too.)* *(Second instance of the same class, in the UPDATE shape: 2026-09-25, TASK-124/#1898 — the write was `findByIdAndUpdate(…, { $set: { 'config.webhookUrlRef': ref } })` on the connect route (`routes/integrations.ts`) and the same shape on the backfill (`services/discordService.ts`), and `config` is a **nested path of a strict schema** — rule 34's `config`: `schema.path('config')` is undefined, `schema.nested.config` is true, `schema.get('strict')` is true — which did not declare the path, so the write was stripped in silence. (The backfill's drop is *derived*, not observed: same schema, same `$set`, and the two writers landed in the same PR, so the declaration fixed both before either merged.) **The split worth naming, because the obvious grep points the wrong way: a write through the Model is stripped by strict mode, a raw-collection write is not.** The only `updateOne` in this change is the migration script's, and it runs on `mongoose.connection.collection()` — strict mode never touches that, so grepping `updateOne` finds the one writer that cannot drop. Same tell, same instrument: the route's own suite stayed green on inline-built fixtures, and the assertion that saw the ref missing is the one that drives the create route against a `MongoMemoryServer`-backed model and reads the row back with `.lean()` (`backend/__tests__/unit/routes/integrations.discordTokenCopy.test.js`). The fix is one declaration — `webhookUrlRef: String` in that `config` nested path in `backend/models/Integration.ts` — carried by the same PR as the assertion, which is why this instance merges **after** #1898; at the rev this was earned, grepping `webhookUrlRef` across the backend returned nothing at all. Two things worth carrying forward: the drop is silent in a way a validation error is not, so the reader simply takes its fallback branch and reports the connector as unavailable; and of the five suites that name the ref, the other four mock `Integration` or seed the ref themselves, so none of them writes it through the route — a mocked writer cannot see the drop at all.)* 25. **Enumerate a guard set by its refusal code, not by its predicate.** Guards that refuse the same operation for the same stated reason are frequently implemented by *different mechanisms*, so a grep for the predicate finds only the ones that happen to share it — and a set you believe is closed is precisely what makes a wrong statement about reachability sound. The mechanical form: grep the **refusal code or status value the client branches on** (frontends catch these by string, which is what makes the code the stable identity), read every hit, and only then say whether a path does or does not reach a gate. A shared error code enumerates a gate set; a predicate enumerates a mechanism. *(Earned: 2026-09-18, #1734 — I grepped `isCloudRuntime`, found exactly two call sites, and told a reviewer "the gate lives only in `routes/registry/install.ts`." The refusal code `cloud_agents_not_entitled` has three sites: `registry/install.ts` and `registry/provision.ts` gate via `isCloudRuntime`, while `personas.ts` gates runtime-agnostically on role + `entitlements.cloudAgents` before calling `hirePersona`. So the hire path was gated all along, and the incomplete map had already been used to argue which paths a ruling had to cover.)* From 720e6d21ee7eccdd57a733af0e736f85cc019c97 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 25 Sep 2026 14:19:00 -0700 Subject: [PATCH 4/5] docs(review): drop the false pre-merge grep from rule 24's second instance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit wren's third correction: the instance was earned ON #1898, and there `webhookUrlRef` appears in 15 backend files, not zero. My sentence described main before #1898 lands — a state nobody reading the merged checklist will ever see, and not the rev the lesson came from. Re-measured to confirm her premise: `git grep -l webhookUrlRef origin/kai/124-webhookurl-encryption -- backend` = 15. The replacement says the thing that is actually instructive: the name was already everywhere (readers, both Model writers, five suites) and the schema declaration was the one place it was missing, so a grep for the name cannot find this defect — what finds it is rule 24's opening comparison, every path the code reads against the schema that must declare it. Docs-only, one line. --- docs/development/review-checklist.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/development/review-checklist.md b/docs/development/review-checklist.md index f33cf7ee4a..c1ac23689f 100644 --- a/docs/development/review-checklist.md +++ b/docs/development/review-checklist.md @@ -80,7 +80,7 @@ And then note where the fix landed. Someone hit the audience gap, felt it, and b 23. **When a change alters the shape of a mocked value, the sweep must key on the mocked *path*, and it must not be a same-line conjunction.** A route's query chain changing shape (`find()` → `find().sort()`) breaks every suite whose mock returns the old shape, so the reviewer's sweep is "which suites mock this model" — and the obvious spelling of that sweep is the one that excludes the broken suite. Three populations over `backend/__tests__/**`, measured at `584442e9`: `jest.mock(.*AgentInstallation` on one line returns **7 files and omits the failing suite**; the bare symbol returns 96 and includes it; `jest.mock('…/models/AgentRegistry'` returns **74** and reproduces the failure. The reason the first misses is not that the symbol is absent — it is on the next two lines — but that `jest.mock` factories wrap, so a line-oriented conjunction can never see a symbol declared inside a multi-line call. **Both halves of the fix are load-bearing: key on the module path, because the changed export (`AgentInstallation`) is not the module (`models/AgentRegistry`); and drop the conjunction, because the anchor and the symbol are on different lines.** Generalises past mocks: any sweep written as `.*` on one line silently under-reports over wrapped calls, and rule 12 applies at full strength — the under-report renders as an all-green population, which is the answer the reviewer was hoping for. *(Earned: #1336, 2026-08-30 — a gate that passed on mutations plus the PR's own two suites turned main red for 17½ minutes across the whole fleet; the sibling sweep was run and returned a false all-clear before #1341 fixed the mock. Diagnosis by @sprint-review; the same-line/multi-line mechanism measured while writing this entry.)* -24. **A fixture that builds its own document proves the reader can read a field — never that the field can be there.** Tests written against a hand-constructed object exercise the *read* and skip the *write*, and the write is exactly where a strict schema silently drops what the reader expects: Mongoose strips undeclared paths out of a `strict` subdocument, and `findOneAndUpdate` does not run validators — so a seeder can persist an enum-invalid value while the field beside it never lands at all. The result is the worst pairing: a green test, and a reader whose preferred branch has never once fired on real data. The fallback usually hides it, because a fallback is what makes the undefined field survivable and the defect invisible. **The check is two greps and one round-trip:** for each field the code reads cross-layer, confirm the writing schema declares that exact path; then confirm at least one test round-trips a document through the real model (`new Model(…).toObject()`, or a real upsert against memory Mongo) rather than constructing the shape inline. If every test for a read path builds its own object, the path is unproven however many assertions it carries — and mutating the schema will not redden it. *(Earned: #1734/TASK-043 — `routes/registry/install.ts` derived a runtime type from `manifest.runtime.runtimeType`, and `ManifestRuntimeSchema` had no such path while being `strict`, so the field was dropped on every write: `new AgentRegistry({manifest:{runtime:{type:'native',runtimeType:'native'}}}).toObject().manifest.runtime` returns `{type:'native',connection:'mcp'}`, and `schema.path('manifest.runtime.runtimeType')` is `undefined`. The route's own test stayed green because its fixture was a plain object; the seeder's enum-invalid `type:'native'` persisted precisely because `findOneAndUpdate` skips validators; and the fallback had been dead on every row ever written. The fix that closes it is the same shape everywhere: declare the path, route every writer through one builder, and round-trip the fixture through the model so deleting the schema path fails the route test too.)* *(Second instance of the same class, in the UPDATE shape: 2026-09-25, TASK-124/#1898 — the write was `findByIdAndUpdate(…, { $set: { 'config.webhookUrlRef': ref } })` on the connect route (`routes/integrations.ts`) and the same shape on the backfill (`services/discordService.ts`), and `config` is a **nested path of a strict schema** — rule 34's `config`: `schema.path('config')` is undefined, `schema.nested.config` is true, `schema.get('strict')` is true — which did not declare the path, so the write was stripped in silence. (The backfill's drop is *derived*, not observed: same schema, same `$set`, and the two writers landed in the same PR, so the declaration fixed both before either merged.) **The split worth naming, because the obvious grep points the wrong way: a write through the Model is stripped by strict mode, a raw-collection write is not.** The only `updateOne` in this change is the migration script's, and it runs on `mongoose.connection.collection()` — strict mode never touches that, so grepping `updateOne` finds the one writer that cannot drop. Same tell, same instrument: the route's own suite stayed green on inline-built fixtures, and the assertion that saw the ref missing is the one that drives the create route against a `MongoMemoryServer`-backed model and reads the row back with `.lean()` (`backend/__tests__/unit/routes/integrations.discordTokenCopy.test.js`). The fix is one declaration — `webhookUrlRef: String` in that `config` nested path in `backend/models/Integration.ts` — carried by the same PR as the assertion, which is why this instance merges **after** #1898; at the rev this was earned, grepping `webhookUrlRef` across the backend returned nothing at all. Two things worth carrying forward: the drop is silent in a way a validation error is not, so the reader simply takes its fallback branch and reports the connector as unavailable; and of the five suites that name the ref, the other four mock `Integration` or seed the ref themselves, so none of them writes it through the route — a mocked writer cannot see the drop at all.)* +24. **A fixture that builds its own document proves the reader can read a field — never that the field can be there.** Tests written against a hand-constructed object exercise the *read* and skip the *write*, and the write is exactly where a strict schema silently drops what the reader expects: Mongoose strips undeclared paths out of a `strict` subdocument, and `findOneAndUpdate` does not run validators — so a seeder can persist an enum-invalid value while the field beside it never lands at all. The result is the worst pairing: a green test, and a reader whose preferred branch has never once fired on real data. The fallback usually hides it, because a fallback is what makes the undefined field survivable and the defect invisible. **The check is two greps and one round-trip:** for each field the code reads cross-layer, confirm the writing schema declares that exact path; then confirm at least one test round-trips a document through the real model (`new Model(…).toObject()`, or a real upsert against memory Mongo) rather than constructing the shape inline. If every test for a read path builds its own object, the path is unproven however many assertions it carries — and mutating the schema will not redden it. *(Earned: #1734/TASK-043 — `routes/registry/install.ts` derived a runtime type from `manifest.runtime.runtimeType`, and `ManifestRuntimeSchema` had no such path while being `strict`, so the field was dropped on every write: `new AgentRegistry({manifest:{runtime:{type:'native',runtimeType:'native'}}}).toObject().manifest.runtime` returns `{type:'native',connection:'mcp'}`, and `schema.path('manifest.runtime.runtimeType')` is `undefined`. The route's own test stayed green because its fixture was a plain object; the seeder's enum-invalid `type:'native'` persisted precisely because `findOneAndUpdate` skips validators; and the fallback had been dead on every row ever written. The fix that closes it is the same shape everywhere: declare the path, route every writer through one builder, and round-trip the fixture through the model so deleting the schema path fails the route test too.)* *(Second instance of the same class, in the UPDATE shape: 2026-09-25, TASK-124/#1898 — the write was `findByIdAndUpdate(…, { $set: { 'config.webhookUrlRef': ref } })` on the connect route (`routes/integrations.ts`) and the same shape on the backfill (`services/discordService.ts`), and `config` is a **nested path of a strict schema** — rule 34's `config`: `schema.path('config')` is undefined, `schema.nested.config` is true, `schema.get('strict')` is true — which did not declare the path, so the write was stripped in silence. (The backfill's drop is *derived*, not observed: same schema, same `$set`, and the two writers landed in the same PR, so the declaration fixed both before either merged.) **The split worth naming, because the obvious grep points the wrong way: a write through the Model is stripped by strict mode, a raw-collection write is not.** The only `updateOne` in this change is the migration script's, and it runs on `mongoose.connection.collection()` — strict mode never touches that, so grepping `updateOne` finds the one writer that cannot drop. Same tell, same instrument: the route's own suite stayed green on inline-built fixtures, and the assertion that saw the ref missing is the one that drives the create route against a `MongoMemoryServer`-backed model and reads the row back with `.lean()` (`backend/__tests__/unit/routes/integrations.discordTokenCopy.test.js`). The fix is one declaration — `webhookUrlRef: String` in that `config` nested path in `backend/models/Integration.ts` — and the tell is worth spelling out, because it defeats the obvious instrument: at the rev this was earned the name already appeared in **15 backend files** (readers, both Model writers, five suites), and the schema declaration was the one place it did not, so a grep for the name cannot find this defect. What finds it is the comparison this rule opens with — every path the code reads, against the schema that must declare it. The declaration and the assertion both land with #1898, which is why this instance merges **after** it. Two things worth carrying forward: the drop is silent in a way a validation error is not, so the reader simply takes its fallback branch and reports the connector as unavailable; and of the five suites that name the ref, the other four mock `Integration` or seed the ref themselves, so none of them writes it through the route — a mocked writer cannot see the drop at all.)* 25. **Enumerate a guard set by its refusal code, not by its predicate.** Guards that refuse the same operation for the same stated reason are frequently implemented by *different mechanisms*, so a grep for the predicate finds only the ones that happen to share it — and a set you believe is closed is precisely what makes a wrong statement about reachability sound. The mechanical form: grep the **refusal code or status value the client branches on** (frontends catch these by string, which is what makes the code the stable identity), read every hit, and only then say whether a path does or does not reach a gate. A shared error code enumerates a gate set; a predicate enumerates a mechanism. *(Earned: 2026-09-18, #1734 — I grepped `isCloudRuntime`, found exactly two call sites, and told a reviewer "the gate lives only in `routes/registry/install.ts`." The refusal code `cloud_agents_not_entitled` has three sites: `registry/install.ts` and `registry/provision.ts` gate via `isCloudRuntime`, while `personas.ts` gates runtime-agnostically on role + `entitlements.cloudAgents` before calling `hirePersona`. So the hire path was gated all along, and the incomplete map had already been used to argue which paths a ruling had to cover.)* From 87f9b8cba2a8d472cda08e7630029ef31ee72fe4 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 25 Sep 2026 14:23:36 -0700 Subject: [PATCH 5/5] =?UTF-8?q?docs(review):=20rule=2024's=20second=20inst?= =?UTF-8?q?ance=20=E2=80=94=20drop=20the=20count,=20fix=20the=20referent?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit wren's two swaps, exactly as given: "at the rev this was earned the name already appeared in **15 backend files**" -> "the name already appeared across the backend" "the comparison this rule opens with" -> "the comparison this rule's check opens with" She is right on the second: the rule opens with its thesis ("A fixture that builds its own document proves the reader can read a field"), and the comparison is the first step of its check ("The check is two greps and one round-trip: for each field the code reads cross-layer, confirm the writing schema declares that exact path"). Nothing else changed: no other path, no other word on the line. --- docs/development/review-checklist.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/development/review-checklist.md b/docs/development/review-checklist.md index c1ac23689f..55067c62ae 100644 --- a/docs/development/review-checklist.md +++ b/docs/development/review-checklist.md @@ -80,7 +80,7 @@ And then note where the fix landed. Someone hit the audience gap, felt it, and b 23. **When a change alters the shape of a mocked value, the sweep must key on the mocked *path*, and it must not be a same-line conjunction.** A route's query chain changing shape (`find()` → `find().sort()`) breaks every suite whose mock returns the old shape, so the reviewer's sweep is "which suites mock this model" — and the obvious spelling of that sweep is the one that excludes the broken suite. Three populations over `backend/__tests__/**`, measured at `584442e9`: `jest.mock(.*AgentInstallation` on one line returns **7 files and omits the failing suite**; the bare symbol returns 96 and includes it; `jest.mock('…/models/AgentRegistry'` returns **74** and reproduces the failure. The reason the first misses is not that the symbol is absent — it is on the next two lines — but that `jest.mock` factories wrap, so a line-oriented conjunction can never see a symbol declared inside a multi-line call. **Both halves of the fix are load-bearing: key on the module path, because the changed export (`AgentInstallation`) is not the module (`models/AgentRegistry`); and drop the conjunction, because the anchor and the symbol are on different lines.** Generalises past mocks: any sweep written as `.*` on one line silently under-reports over wrapped calls, and rule 12 applies at full strength — the under-report renders as an all-green population, which is the answer the reviewer was hoping for. *(Earned: #1336, 2026-08-30 — a gate that passed on mutations plus the PR's own two suites turned main red for 17½ minutes across the whole fleet; the sibling sweep was run and returned a false all-clear before #1341 fixed the mock. Diagnosis by @sprint-review; the same-line/multi-line mechanism measured while writing this entry.)* -24. **A fixture that builds its own document proves the reader can read a field — never that the field can be there.** Tests written against a hand-constructed object exercise the *read* and skip the *write*, and the write is exactly where a strict schema silently drops what the reader expects: Mongoose strips undeclared paths out of a `strict` subdocument, and `findOneAndUpdate` does not run validators — so a seeder can persist an enum-invalid value while the field beside it never lands at all. The result is the worst pairing: a green test, and a reader whose preferred branch has never once fired on real data. The fallback usually hides it, because a fallback is what makes the undefined field survivable and the defect invisible. **The check is two greps and one round-trip:** for each field the code reads cross-layer, confirm the writing schema declares that exact path; then confirm at least one test round-trips a document through the real model (`new Model(…).toObject()`, or a real upsert against memory Mongo) rather than constructing the shape inline. If every test for a read path builds its own object, the path is unproven however many assertions it carries — and mutating the schema will not redden it. *(Earned: #1734/TASK-043 — `routes/registry/install.ts` derived a runtime type from `manifest.runtime.runtimeType`, and `ManifestRuntimeSchema` had no such path while being `strict`, so the field was dropped on every write: `new AgentRegistry({manifest:{runtime:{type:'native',runtimeType:'native'}}}).toObject().manifest.runtime` returns `{type:'native',connection:'mcp'}`, and `schema.path('manifest.runtime.runtimeType')` is `undefined`. The route's own test stayed green because its fixture was a plain object; the seeder's enum-invalid `type:'native'` persisted precisely because `findOneAndUpdate` skips validators; and the fallback had been dead on every row ever written. The fix that closes it is the same shape everywhere: declare the path, route every writer through one builder, and round-trip the fixture through the model so deleting the schema path fails the route test too.)* *(Second instance of the same class, in the UPDATE shape: 2026-09-25, TASK-124/#1898 — the write was `findByIdAndUpdate(…, { $set: { 'config.webhookUrlRef': ref } })` on the connect route (`routes/integrations.ts`) and the same shape on the backfill (`services/discordService.ts`), and `config` is a **nested path of a strict schema** — rule 34's `config`: `schema.path('config')` is undefined, `schema.nested.config` is true, `schema.get('strict')` is true — which did not declare the path, so the write was stripped in silence. (The backfill's drop is *derived*, not observed: same schema, same `$set`, and the two writers landed in the same PR, so the declaration fixed both before either merged.) **The split worth naming, because the obvious grep points the wrong way: a write through the Model is stripped by strict mode, a raw-collection write is not.** The only `updateOne` in this change is the migration script's, and it runs on `mongoose.connection.collection()` — strict mode never touches that, so grepping `updateOne` finds the one writer that cannot drop. Same tell, same instrument: the route's own suite stayed green on inline-built fixtures, and the assertion that saw the ref missing is the one that drives the create route against a `MongoMemoryServer`-backed model and reads the row back with `.lean()` (`backend/__tests__/unit/routes/integrations.discordTokenCopy.test.js`). The fix is one declaration — `webhookUrlRef: String` in that `config` nested path in `backend/models/Integration.ts` — and the tell is worth spelling out, because it defeats the obvious instrument: at the rev this was earned the name already appeared in **15 backend files** (readers, both Model writers, five suites), and the schema declaration was the one place it did not, so a grep for the name cannot find this defect. What finds it is the comparison this rule opens with — every path the code reads, against the schema that must declare it. The declaration and the assertion both land with #1898, which is why this instance merges **after** it. Two things worth carrying forward: the drop is silent in a way a validation error is not, so the reader simply takes its fallback branch and reports the connector as unavailable; and of the five suites that name the ref, the other four mock `Integration` or seed the ref themselves, so none of them writes it through the route — a mocked writer cannot see the drop at all.)* +24. **A fixture that builds its own document proves the reader can read a field — never that the field can be there.** Tests written against a hand-constructed object exercise the *read* and skip the *write*, and the write is exactly where a strict schema silently drops what the reader expects: Mongoose strips undeclared paths out of a `strict` subdocument, and `findOneAndUpdate` does not run validators — so a seeder can persist an enum-invalid value while the field beside it never lands at all. The result is the worst pairing: a green test, and a reader whose preferred branch has never once fired on real data. The fallback usually hides it, because a fallback is what makes the undefined field survivable and the defect invisible. **The check is two greps and one round-trip:** for each field the code reads cross-layer, confirm the writing schema declares that exact path; then confirm at least one test round-trips a document through the real model (`new Model(…).toObject()`, or a real upsert against memory Mongo) rather than constructing the shape inline. If every test for a read path builds its own object, the path is unproven however many assertions it carries — and mutating the schema will not redden it. *(Earned: #1734/TASK-043 — `routes/registry/install.ts` derived a runtime type from `manifest.runtime.runtimeType`, and `ManifestRuntimeSchema` had no such path while being `strict`, so the field was dropped on every write: `new AgentRegistry({manifest:{runtime:{type:'native',runtimeType:'native'}}}).toObject().manifest.runtime` returns `{type:'native',connection:'mcp'}`, and `schema.path('manifest.runtime.runtimeType')` is `undefined`. The route's own test stayed green because its fixture was a plain object; the seeder's enum-invalid `type:'native'` persisted precisely because `findOneAndUpdate` skips validators; and the fallback had been dead on every row ever written. The fix that closes it is the same shape everywhere: declare the path, route every writer through one builder, and round-trip the fixture through the model so deleting the schema path fails the route test too.)* *(Second instance of the same class, in the UPDATE shape: 2026-09-25, TASK-124/#1898 — the write was `findByIdAndUpdate(…, { $set: { 'config.webhookUrlRef': ref } })` on the connect route (`routes/integrations.ts`) and the same shape on the backfill (`services/discordService.ts`), and `config` is a **nested path of a strict schema** — rule 34's `config`: `schema.path('config')` is undefined, `schema.nested.config` is true, `schema.get('strict')` is true — which did not declare the path, so the write was stripped in silence. (The backfill's drop is *derived*, not observed: same schema, same `$set`, and the two writers landed in the same PR, so the declaration fixed both before either merged.) **The split worth naming, because the obvious grep points the wrong way: a write through the Model is stripped by strict mode, a raw-collection write is not.** The only `updateOne` in this change is the migration script's, and it runs on `mongoose.connection.collection()` — strict mode never touches that, so grepping `updateOne` finds the one writer that cannot drop. Same tell, same instrument: the route's own suite stayed green on inline-built fixtures, and the assertion that saw the ref missing is the one that drives the create route against a `MongoMemoryServer`-backed model and reads the row back with `.lean()` (`backend/__tests__/unit/routes/integrations.discordTokenCopy.test.js`). The fix is one declaration — `webhookUrlRef: String` in that `config` nested path in `backend/models/Integration.ts` — and the tell is worth spelling out, because it defeats the obvious instrument: the name already appeared across the backend (readers, both Model writers, five suites), and the schema declaration was the one place it did not, so a grep for the name cannot find this defect. What finds it is the comparison this rule's check opens with — every path the code reads, against the schema that must declare it. The declaration and the assertion both land with #1898, which is why this instance merges **after** it. Two things worth carrying forward: the drop is silent in a way a validation error is not, so the reader simply takes its fallback branch and reports the connector as unavailable; and of the five suites that name the ref, the other four mock `Integration` or seed the ref themselves, so none of them writes it through the route — a mocked writer cannot see the drop at all.)* 25. **Enumerate a guard set by its refusal code, not by its predicate.** Guards that refuse the same operation for the same stated reason are frequently implemented by *different mechanisms*, so a grep for the predicate finds only the ones that happen to share it — and a set you believe is closed is precisely what makes a wrong statement about reachability sound. The mechanical form: grep the **refusal code or status value the client branches on** (frontends catch these by string, which is what makes the code the stable identity), read every hit, and only then say whether a path does or does not reach a gate. A shared error code enumerates a gate set; a predicate enumerates a mechanism. *(Earned: 2026-09-18, #1734 — I grepped `isCloudRuntime`, found exactly two call sites, and told a reviewer "the gate lives only in `routes/registry/install.ts`." The refusal code `cloud_agents_not_entitled` has three sites: `registry/install.ts` and `registry/provision.ts` gate via `isCloudRuntime`, while `personas.ts` gates runtime-agnostically on role + `entitlements.cloudAgents` before calling `hirePersona`. So the hire path was gated all along, and the incomplete map had already been used to argue which paths a ruling had to cover.)*