refactor(metadata-protocol): move the query validation out of protocol.ts into protocol-query-validation.ts (#25467 cut 3) (#25538)

<!-- claude-projects-attribution: {"github_login":"os-sales"} -->
_Requested by **Sales** · [project
thread](https://claude.ai/code/project/chan_01D5bWFY3rFzgJcytD5gD4Uu?thread=cmsg_01D5bWFY3rFzgJcytD5gD4Uu2dTVig4NQQadyC11Fs5BFZ)_

Part of #25467
Clause-②: no
Metadata-protocol: none
Ruling-ref: 6270097840

Cut 3 of the `protocol.ts` split (#24586 ruling A, recorded in
`6270097840`). Cut 1 landed as #25494 and cut 2 as #25518. The claim is
`6373234211`; the seat agreed the scope and its conditions before it.
Nineteen PRIVATE `ObjectStackProtocolImplementation` methods, the query
validation, move unedited into a new module,
`packages/metadata-protocol/src/protocol-query-validation.ts`: the
section `:12797`–`:14502` on base `b2411c001d` (1,706 lines, 382 of them
from the last 30 days).

It is move-only, and the card stays open for the later cuts. Branch:
`claude/project-thread-uasfag`, the harness-pinned branch for this
session, restarted from `origin/main` at `b2411c001d` after cut 2 merged
and rebased onto `f38832be9c` (which touches nothing in this package).
It names no issue, so the claim comment is the identity record.

**Serial with the file's in-flight PRs.** At claim, none of the 9 open
PRs touched `protocol.ts` (each PR's diff against its merge base read;
the Version Packages PR #24876 touches only `CHANGELOG.md` and
`package.json` in this package). Re-measured at 20:54Z, just before this
PR was opened, on all 12 open PRs: one new PR, #25533 (opened 20:29Z),
edits `protocol.ts` in two places, a docblock line in `publishDrafts`'
request type (`:26260`) and one spread in its build-probe call
(`:27144`). Neither is in or next to the moved region (the
query-validation section `:12797`–`:14502` and the module-level
declarations at `:392`–`:5348`), and `git merge-tree` of this head with
#25533's head is clean, so this cut does not yield to it. No open PR
touches `second-object-read-exposure.pin.test.ts`. `origin/main`
(`74496c64b0`) has not touched either file since this branch's base, and
`git merge-tree` with it is clean. Whichever of the two lands second
changes `protocol.ts`'s byte offsets under the other (see the series
note at the end).

## What moves

The 19 methods, all `private` (so the published `.d.ts` prints no type
for any of them, and none is on the series' skip list of public members
with inferred return types):

`assertObjectRegistered`, `resolveQueryFields`,
`assertQueryParamsAreFields`, `assertFilterFieldsExist`,
`assertSortFieldsExist`, `assertProjectionFieldsExist`,
`assertExpandTargetsExist`, `assertSearchFieldsAreSearchable`,
`assertGroupByFieldsExist`, `assertAggregationFieldsExist`,
`assertNoInternalFieldEvaluated`, `refuseInternalInFilter`,
`refuseUnservedRelationTarget`, `refuseInternalInExpand`,
`servedExpand`, `servesExpansionTarget`, `orderByFieldNames`,
`groupByFieldNames`, `refuseInternalNames`.

## The shape (#23050 shape B, as cuts 1–2 apply it)

- The class keeps every method with its name, docblock, signature and
position. Each body is now one host literal plus `fn.call(host, …)`,
marked `[#25467 cut 3]`; a body that reads no member calls its function
directly. None of the 19 was `async`, so no stub changes its modifiers.
- `QueryValidationHost` types every member as the class member itself
(`ObjectStackProtocolImplementation['m']`). Each moved function types
its `this` as the `Pick` of the host over exactly the members its body
reads (measured by AST), and each stub annotates its literal as
`ThisParameterType<typeof fn>`. There is no cast, and no member becomes
public or protected.
- The members read, measured by AST: the field `engine`, as a getter so
each read happens when the body makes it; and the methods
`assertFilterFieldsExist`, `groupByFieldNames`, `orderByFieldNames`,
`refuseInternalInExpand`, `refuseInternalInFilter`,
`refuseInternalNames`, `refuseUnservedRelationTarget`,
`resolveQueryFields`, `servedExpand`, `servesExpansionTarget`, as
late-bound closures (`(...a) => this.m(...a)`). The recursive
`refuseInternal*` walks therefore still recurse through the protocol,
and a spy or double set on the instance is still the one the body
reaches.

## Module-level declarations, by reader

Each declaration the moved bodies read was checked by an AST walk over
the whole of base `protocol.ts`: every identifier reference to it
outside its own declaration, attributed to the enclosing class member or
top-level statement.

**Move with the bodies, byte-identical and unexported.** Each has no
reader outside the moved bodies and the other moved declarations (0
outside references each):

| Declaration | Base line | Readers |
|---|---|---|
| `nestedRelationRoute` | `:392` | moved bodies only |
| `warnedNoRegistryForDataGate` (`let`, mutable) | `:536` |
`assertObjectRegistered` only: its one read (`:12864`) and its one write
(`:12865`). The state moves whole; it is not copied, so there is still
exactly one flag. |
| `AGGREGATION_FUNCTIONS` | `:4489` | moved bodies only |
| `DATE_GRANULARITIES` | `:4490` | moved bodies only |
| `UNMATERIALIZED_SORT_TYPES` | `:4892` | moved bodies only |
| `invalidQueryError` | `:4988` | moved bodies only |
| `suggestQueryParam` | `:5131` | moved bodies only |
| `collectFilterFieldKeys` | `:5217` | moved bodies only |
| `INTERNAL_FIELD_WALK_DEPTH` | `:5321` | moved bodies only |
| `EXPANSION_TARGET_OPERATION` | `:5337` | moved bodies only |
| `RELATION_CONDITION_TARGET_OPERATION` | `:5348` | moved bodies only |

Their initialisers read only package imports (`AggregationFunction`,
`DateGranularity`) or nothing, so neither module reads the other's
bindings while they are first evaluated.

**Comments around the moved declarations.** Each moved declaration
carries exactly its own docblock, byte-identical. Two comments sat above
them at base and were decided one by one:

- Base `:330`–`:383` is `canonicalMetaType`'s docblock ("The ONE
canonical spelling of a metadata type…"), which sits above
`nestedRelationRoute` only because #20802 inserted that function between
the docblock and `canonicalMetaType` (`:399`). It documents a function
that stays, so it stays in `protocol.ts`; with `nestedRelationRoute`
gone it sits directly above `canonicalMetaType` again. Only
`nestedRelationRoute`'s own docblock (`:384`–`:391`) moves.
- Base `:523`–`:528` is a six-line `[#5488]` tombstone
(`PUBLISH_DRAFTS_NAMESPACE_REMEDY` retired) that is the leading comment
of `warnedNoRegistryForDataGate`. It moves with that declaration,
unedited. It was measured both ways: left in `protocol.ts`, it shifts
the file's byte offsets so that the emitted `.d.ts` splices an unrelated
`protocol.ts` docblock into `getMetaTypes`' inferred return type
(normalised hash `fd5f361b…` or `9b65c545…` depending on where the
docblock above sits), the same splice cut 2 measured on `getDiscovery`.
Moved, the `.d.ts` meets the bar (measured in the next section). See
"Series note" at the end.

**Stay in `protocol.ts` and gain `export` in place**, the only edit to
each, imported back by the module:

| Declaration | Read by code that stays |
|---|---|
| `collectFilterReads` | `collectStoredMetadataFilterFields` |
| `invalidSortError` | `normalizeSortNodes`,
`invalidSortDirectionKeyError` |
| `isObjectOfDisabledPackage`, `objectPackageDisabledError` |
`readMetaItemFace`, `getMetaItemLayered` |
| `isPlainRecord` | `storableEqual`, `soleMovedFrom`,
`buildStorableViewNode` |
| `suggestFieldName`, `nearMissKey`, `QUERY_PARAM_NEAR_MISS`,
`ODATA_SPELLING`, `FILTER_LOGICAL_KEYS` | none: see below |

**Correction to the claim.** The claim grouped all ten as read by code
that stays. Measured by the same AST walk, five of them are read only by
code that moves, directly or through each other: `suggestFieldName` (7
reads in the moved methods, 1 in `suggestQueryParam`), `nearMissKey`
(`suggestQueryParam`, `suggestFieldName`), `QUERY_PARAM_NEAR_MISS` and
`ODATA_SPELLING` (`suggestQueryParam`), `FILTER_LOGICAL_KEYS`
(`collectFilterFieldKeys`); `editDistance`, which stays unexported, is
read only by `suggestFieldName`. They stay here anyway, exported in
place: staying is the direction the series' rule always allows, the six
form one near-miss vocabulary interleaved with `normalizeSortNodes`
(which stays), and three test comments name "`protocol.ts`'s own
`FILTER_LOGICAL_KEYS`". Moving the six is a pure move with no reader to
migrate, left for a later cut if the seat wants it, so this PR's
measured layout is not reopened.

`src/index.ts` exports from `./protocol.js` by name, so none of the ten
is published (measured below).

**Imports.** `protocol.ts` drops the names that lose every reader
(`noUnusedLocals`): `objectNotFoundError` and `relationConditionSites`
from `@objectstack/core`, `collectInternalWriteResponseFields` from
`./write-response-internal-fields.js`, and ten names from
`@objectstack/spec/data`. The module imports them from the same
specifiers. `protocol.ts` gains one statement, the import of the new
module.

## The published `.d.ts`

Under the series bar (ruling `6348891160`), measured on base
`b2411c001d` and on the head. Both were built against the same
dependency tree: `turbo run build
--filter=@objectstack/metadata-protocol...` at base (13/13), then `pnpm
--filter @objectstack/metadata-protocol build` at head.

- Raw sha256 of `dist/index.d.ts` and `dist/index.d.cts`: base
`af5c30c9cb32b251f9a1b075e6347f372fe4be2561bee9c97731155b5d0a62a7` (both
files), head
`a3e7fd469c1db0353692b3dcdab14820d843f65ac2e9e896cf2e6d63bf9890bc` (both
files).
- The raw diff is one swapped line (`2419`), the discovery
`capabilities` key union: `… "search" | "i18n" | "notifications" |
"export" | …` at base and `… "search" | "export" | "i18n" |
"notifications" | …` at head. That is a union-member order swap, the
kind the ruling names. The file size is identical (769,048 bytes).
- Normalised sha256, base and head (both files):
`5b98a8e857554959f7561563d6a0742110c5d3d8ad6119065ed5a69b0bc06f95`, the
same value as at cuts 1 and 2. The normaliser, run from
`packages/metadata-protocol`:

```sh
node -e 'const s=require("fs").readFileSync(process.argv[1],"utf8");process.stdout.write(s.replace(/"[^"\n]*"(?:\s*\|\s*"[^"\n]*")+/g,r=>r.split(/\s*\|\s*/).sort().join(" | ")))' dist/index.d.ts | sha256sum
```

- Positive control on a copy of the head `dist/index.d.ts`: line 1348,
`private engine;`, made public (`engine;`), gives raw
`2ee8f1cbc4b49721d0967bbcddd906868775f4a889fda8d1daffa8ba2804f0a8` and
normalised
`f40390e5c426e0c6e24523cd6908726d3f3cab87f9a4ac36df20992bec04da95`. Both
differ from the real hashes.
- Export tables: the ESM and CJS bundles each export the same 78 names
at base and head, loaded at runtime (`import()` and `require` of the
built bundles). The sorted list hashes
`e47ac7c753bcb8bee1e79c1a5f79f7394cf030bd526de81ea8bcbfcb71fa024a` on
all four, as at cuts 1 and 2. None of the ten newly exported
declarations appears.

No changeset: `skip-changeset`, per ruling `6348891160`.

## Proof of move-only

- **Bodies and declarations, by AST token** (recursive `getChildren`
leaves): each moved function's parameters (less the added `this`),
return type and body have the same token sequence as the base method,
and each body line is the base line with four leading spaces removed.
Each moved declaration has the same tokens and its own docblock
byte-identical (`nestedRelationRoute` carries one docblock at head where
base attached two; the other is `canonicalMetaType`'s, above). 6,585
tokens compared (6,149 in the methods), 0 differ. Negative control: one
changed token makes the check report a difference.
- **Class side**: the class has 298 members at base and head. The 279
that did not move are byte-identical to base. Each of the 19 stubs keeps
its docblock and signature byte-identical.
- **Top level of `protocol.ts`**: 232 of the 256 base statements other
than the class are byte-identical with their leading comments. The other
24 are the 11 moved declarations, the 10 that gain `export`, the 2
narrowed import statements, and `canonicalMetaType`, whose code is
byte-identical and whose leading comment is now its own docblock
(above). The one new statement is the import of the new module.
- **Size**: +1,544 / −1,235 over 4 files, 2,779 changed lines, under the
3,000-line human-merge bar (`dispatch-gates` measures it: "under").
`protocol.ts` goes from 32,309 to 31,223 lines;
`protocol-query-validation.ts` has 1,393.

## Readers of the moved lines

- **Second-object read exposure pin**
(`packages/core/src/security/second-object-read-exposure.pin.test.ts`),
found by running the cross-package tests that read `protocol.ts` by
path; not listed in the claim, so disclosed here. It reads source by
path: which files call the reference-target arbiter
(`referenceTargetOf`), the body of each door's decision function, and
each wiring call site. The arbiter call, `servesExpansionTarget`,
`refuseUnservedRelationTarget` and `servedExpand` now live in the new
module, so at head without a migration it fails 3 of 4 tests (an
unclassified caller, a decision function that no longer asks the
decision, and a classified file the matcher no longer fires on). Six
path strings move to `protocol-query-validation.ts`: the classified
caller key, both `decision.file`s, the executes row's `decidedIn`, the
relation row's `wiring.file` (its one call site,
`this.refuseUnservedRelationTarget(`, moved; the match left in
`protocol.ts` would only be the stub's host closure), and the vacuity
control's `servedExpand` body read. The `$expand` row's wiring stays on
`protocol.ts`, where `findData`/`getData` still call
`this.servedExpand(`. No row is added, removed or reworded; 4/4 pass at
head, as at base.
- **Bracketed-refusal pin**
(`protocol.bracketed-refusal-opener-absence.test.ts`). 17 `new Error(`
sites move: 16 in the methods and 1 in `invalidQueryError`. Measured at
both ends: `protocol.ts` 104 → 87, against its floor of 5. The new
module is added to `PRODUCERS` with its measured count as its floor,
`'protocol-query-validation.ts': 17`, as cut 1 listed
`protocol-batch-data.ts`. With the exposure pin's paths above, those are
the only edits outside the move; this one adds exactly one test (the new
`it.each` row), which passes. No moved site opens with a bracketed tag.
- **ADR anchors.** No paragraph of the `protocol.ts` entry in
`scripts/adr-anchors/` describes these bodies, read paragraph by
paragraph for half-matches too: they cover ADR-0029 D9.8/D9.9 hydration
seams, ADR-0119 `batchData`, ADR-0005/0087 view storage and ADR-0126
flow Regime C. So no entry is added.
- One decision letter in the moved text leaves the anchored surface:
ADR-0121 D2, in the `[#5488]` tombstone that moves with
`warnedNoRegistryForDataGate` (above).
- `check:adr-anchors`: 82 → 82 anchored files; decision-letter citations
on the anchored surface 1,969 → 1,968, measured at both ends.
- **Platform checklist.**
`docs/qa/platform-checklist/areas/api-backend.json` (`:514`, `:515`,
`:2007`, `:2855`) and `search.json` (`:202`) anchor `protocol.ts#` moved
methods. The method shells stay, so they still resolve; they are not
edited, as cuts 1 and 2 left theirs. `check-platform-checklist` exits 0
at both ends with identical output.
- **Censuses and ledgers.** The moved text has no `isSystem` read
(`check-system-context-census`: 149 sites at both ends, identical
output), no engine `find`/`findOne`/`count`/`aggregate` call (so
`scripts/query-options-erasure-baseline.json`'s `protocol.ts: 6` is
unaffected), no engine write (`check-tenant-audit-census`: 238 sites at
both ends, identical output, no `--write`), no engine-double contract
seam, and no `lookupArtifactItem(` call (the call-site pin's enumeration
is unaffected).
- **Source scans.** The package's source scans that read `./protocol.ts`
(`register-item`, `lookup-artifact-item` and `view-expansion-reconcile`
call-site pins, `hydrate-overlay-canonical-type`,
`object-registry-write-through-spelling`, `query-transport-dialect`,
`read-verb-canonical-fold`, the two recovery-door pins) all pass at
head; none of their patterns occurs in the moved text.

## Verification

- `tsc --noEmit` (package) exit 0; `eslint` on the four changed files
exit 0.
- **`packages/metadata-protocol` suite.**
  - Base: 21,476 tests (21,457 passed, 0 failed, 19 skipped), 264 files.
  - Head: 21,477 (21,458 passed, 0 failed, 19 skipped), 264 files.
- Compared per test name and status: none lost, none changed status, one
added: the new `PRODUCERS` row, `protocol-query-validation.ts raises no
message opening with a bracketed lowercase tag` (passed).
- **Tests outside the package that read `protocol.ts` by path**, run at
head, all pass:
- `core` `row-serving-door-exposure.pin` and
`second-object-read-exposure.pin` (7, the latter migrated above)
  - `objectql` `core-boundary.ratchet` (2)
- `spec` `metadata-type-registry-allow-org-override-absence` and
`scripts/liveness/evidence` (44)
  - `client` and `rest` `discovery-double-retired-key` (3 + 2)
  - `spec` `check-liveness` exits 0.
- The other files that name `protocol.ts` next to a moved symbol are
prose (comments, `docs/qa` text, a dogfood ledger's `enforcement`
string), read by no code.
- **Hand gates at base and head**, all exit 0 at both:
`check-adr-symbol-anchors` (2,384 anchors resolve at both, identical
output), `check-adr-anchors`, `check-tenant-audit-census`,
`check-system-context-census`, `check-platform-checklist`.
- **Derived gates.** `dispatch-gates --commands --changed` derives 60
gates on the final tree (4 changed paths); all 60 exit 0 and `--ran`
reconciles 60 of 60 with 0 NOT-MEASURED, each run at head with its exit
code. `check-adr-symbol-anchors` and `check-liveness` are not derived
and were run by hand (above). The full list and the `--ran` reconcile
are in the report on this PR.

## Series note: the inferred-return `.d.ts` depends on `protocol.ts`
byte offsets

Measured on this cut: with the same moved code, four placements of the
two comments above give three different `.d.ts` files. The emitter
prints the inferred return types of the public members listed below
(here `getMetaTypes`) with nodes that keep their original positions, and
it reads leading comments for them out of `protocol.ts` at those
positions. So whether a `protocol.ts` docblock is spliced into the
published type depends on where the file's bytes fall, not on what
moved. Any edit to `protocol.ts` can trip it, a cut or not. Each cut in
this series measures it (this PR meets the bar, measured above), and the
carry-forward stands.

## Carry-forward (unchanged from cut 2)

A public member whose return type is inferred stays in `protocol.ts` in
this series; annotating its type would be a signature edit and needs a
ruling. Later cuts skip `getDiscovery`, `getMetaTypes`, `getMetaItems`,
`getMetaItemsForExecution`, `getMetaItem`, `getMetaItemForExecution`,
`getUiView`, `findData`, `getData`, `validateData`, `createData`,
`cloneData`, `updateData`, `deleteData`, `saveMetaItem`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01J8pUyVs5eas1AjMdReJM7C

Co-authored-by: Claude <noreply@anthropic.com>

Author: objectstack-fleet[bot]

packages/core/src/security/second-object-read-exposure.pin.test.ts

@@ -125,18 +125,18 @@ type Classification =
 
 /** Every non-test caller of the arbiter, classified. */
 const CALLERS: Record<string, Classification> = {
-  'packages/metadata-protocol/src/protocol.ts': {
+  'packages/metadata-protocol/src/protocol-query-validation.ts': {
     kind: 'decided',
     read: "the data door's `$expand` (the list, single-record, query and export routes, every level of the tree)",
     operation: 'get',
-    decision: { file: 'packages/metadata-protocol/src/protocol.ts', fn: 'servesExpansionTarget' },
+    decision: { file: 'packages/metadata-protocol/src/protocol-query-validation.ts', fn: 'servesExpansionTarget' },
     wiring: { file: 'packages/metadata-protocol/src/protocol.ts', call: 'this.servedExpand(' },
     pin: 'packages/metadata-protocol/src/protocol.expand-target-exposure.test.ts',
   },
   'packages/objectql/src/engine.ts': {
     kind: 'executes',
     read: "the expansion sub-read (`ObjectQL.expandRelatedRecords`), for the data door's `$expand`",
-    decidedIn: 'packages/metadata-protocol/src/protocol.ts',
+    decidedIn: 'packages/metadata-protocol/src/protocol-query-validation.ts',
     also:
       'the write-path reference-existence probe (`referenceExists`: whether a stored id resolves, no field) and ' +
       'the validation-predicate reads (`resolvePredicateRelated`: a pass/fail verdict, the value never served). ' +
@@ -162,8 +162,8 @@ const CALLERS: Record<string, Classification> = {
     kind: 'decided',
     read: "the data door's nested-relation filter condition (`ObjectQL.lowerRelationConditions` reads the related object to lower it), refused rather than withheld",
     operation: 'list',
-    decision: { file: 'packages/metadata-protocol/src/protocol.ts', fn: 'refuseUnservedRelationTarget' },
-    wiring: { file: 'packages/metadata-protocol/src/protocol.ts', call: 'this.refuseUnservedRelationTarget(' },
+    decision: { file: 'packages/metadata-protocol/src/protocol-query-validation.ts', fn: 'refuseUnservedRelationTarget' },
+    wiring: { file: 'packages/metadata-protocol/src/protocol-query-validation.ts', call: 'this.refuseUnservedRelationTarget(' },
     pin: 'packages/metadata-protocol/src/protocol.relation-condition-target-exposure.test.ts',
   },
   'packages/plugins/plugin-approvals/src/approval-service.ts': {
@@ -395,7 +395,7 @@ describe('[#22661] every read that reaches a second object asks that object its
       expect(mentioning).toContain(declaration);
       expect(callsArbiter(declaration)).toBe(false);
       // The body reader reads the right function: the decision lives in it, not beside it.
-      expect(functionBody('packages/metadata-protocol/src/protocol.ts', 'servedExpand').some((line) => DECISION_CALL.test(line))).toBe(false);
+      expect(functionBody('packages/metadata-protocol/src/protocol-query-validation.ts', 'servedExpand').some((line) => DECISION_CALL.test(line))).toBe(false);
     },
     SCAN_TIMEOUT_MS,
   );