Repository navigation
Commit 70ce802
fix(service-analytics): queryDataset compiles into a request scope and never writes the shared registries (#20380)
Fixes #20356
Clause-②: no
## What this changes
`AnalyticsService.queryDataset` no longer writes the service-wide
`CubeRegistry` or the compiled-dataset registry. It used to register the
dataset's compiled cube under the dataset's name before running the
selection and before any admission was asked, so that name then meant
one request's definition for every later reader until restart.
- **Pure compile.** A new private `compile(dataset)` binds this
service's probes and registers nothing. `registerDataset` (the
configuration door: `AnalyticsServiceConfig.datasets` and embedders) is
`compile` + register, unchanged in behaviour. `queryDataset` calls only
`compile`.
- **A request scope for every name-keyed read.** A small internal
`CubeScope` (`getCube`, `getCompiledDataset`, `register`) is threaded
through the query path. The shared scope reads and writes the
registries. A `queryDataset` call gets a request scope: its own compiled
dataset answers its name, every other name reads the shared scope
read-only, and `register` writes only to the request scope.
- **One body for both.** `query()` is now `queryIn(sharedScope, …)`, and
the `DatasetExecutor` of a dataset call queries through a face whose
`query()` is `queryIn(requestScope, …)`. Every gate is the same code;
only the answer to "which cube does this name mean" differs.
- Comments that described `queryDataset` as a registration door are
corrected in `cube-registry.ts` and `dataset-compiler.ts`.
No new refusal is added. A dataset whose name matches a configured cube
is served from its own definition and no longer meets that cube (triage
note 3).
## Mechanism assumptions, measured
1. **Other registry writes.** `registerDataset` was not the only
request-time write reachable from `queryDataset`. `ensureCube`'s measure
augmentation also registered into the shared registry, through
`DatasetExecutor` → `query()`, when a selection named an undeclared
suffix measure. It now writes into the request scope, which is pinned.
The same two `ensureCube` writes (inference and augmentation) remain
request-time shared writes on the `/analytics/query` and
`/analytics/sql` doors. They are not the dataset door, and they are
reported, not changed (see Acceptance notes). The boot-time writes are
unchanged: `config.cubes` and `config.datasets` in the constructor.
2. **Name-keyed reads on the query path.** Each of these reads the
compiled cube by name, and each now resolves through the call's scope:
- `ensureCube`'s existence and source-field gates;
- `queryObjects`, which is both the object-level admission set and the
read-scope set;
- the strategy context's `getCube` (both strategies' `canHandle`,
`execute` and `generateSql`);
- `getAllowedRelationships` (the NativeSQL join allowlist) and
`getDatasetScope` (both strategies), which read the compiled-dataset
registry.
ObjectQL's `resolveFkAttr` reads no registry, and
`queryCapabilities(cube)` is a config hook. `DatasetExecutor` itself
reads `compiled.cube` directly, but it issues `query({ cube: name })`,
which is why it needs the scoped face.
3. **Doors.** `POST /api/v1/analytics/dataset/query` (`rest-server.ts`)
calls `queryDataset`. `GET /api/v1/analytics/meta`
(`runtime/src/domains/analytics.ts`) calls `getMeta()`, which reads
`cubeRegistry.getAll()`, the shared registry.
4. **Hidden cube.** On `main` nothing reads `Cube.public`: three
producers write it and no reader exists in `service-analytics`,
`runtime` or `rest`. So "stays hidden from meta" is **NOT MEASURABLE on
this tree**. What is pinned is that the registry entry keeps the
author's `public: false` definition (unit test) and that member B's
whole `meta` answer equals B's baseline (route test). Once
`analytics_cube.public` enforcement lands, that same equality asserts
that the cube stays omitted.
## Tests (all on `15e21b98`)
- `src/__tests__/query-dataset-request-scope.test.ts` (new, both
strategy paths, 12 cases). A second caller's `getMeta()` and the exact
driver calls its queries of the configured cube and of the
boot-registered dataset are snapshotted before and after each of these
requests:
- a refused dataset under a configured cube's name (asserts
`PERMISSION_DENIED` / 403, and that the driver never saw the walled
object);
- an admitted one, served from its own object;
- a `public: false` name;
- fresh names, one refused and one admitted with an augmented measure.
The control is the dataset registered at construction, which still
serves and keeps its compiled filter.
-
`packages/qa/dogfood/test/analytics-inline-dataset-isolation.dogfood.test.ts`
(new, `bootStack`, two sign-ups, `sqlite-wasm` + `memory`, 10 cases).
Member B's `meta`, B's authored-cube query and B's saved-dataset query
equal B's baseline after member A's requests: refused (403
`PERMISSION_DENIED`), admitted (200 from A's definition, A's own row
count), hidden-name (200, with no new refusal) and saved-name. The app's
saved dataset is the control.
- `dataset-i18n-label-resolution.test.ts`: the zh-CN meta pin relied on
`queryDataset` registering. It now registers through `registerDataset`
first, and its intent is kept: a zh-CN query leaves the published titles
in the source language.
- `pnpm --filter @objectstack/service-analytics typecheck` passes, and
`vitest run` gives 130 files / 3053 tests, all passing. `pnpm --filter
@objectstack/dogfood typecheck` passes, and the four analytics dogfood
files give 28 tests, all passing.
**Ablations** (each run with the fix committed first, mutation landing
proven on disk by `scripts/ablation-replace.mjs`, and restore proven by
blob equal to HEAD plus an empty `git diff HEAD`):
- **A, request-path registry write restored (unit).** `queryDataset`
calls `registerDataset` again. 10 of 12 go red. The two baselines stay
green.
- **B, admission set read from the shared registry (unit).** The refused
leg and the fresh-name leg go red, because the request resolved instead
of rejecting. This shows that a scope applied to the strategy but not to
the admission set would admit a read of the walled object.
- **A, route level through `dist/`.** Mutate, rebuild, then
`ablation-dist-preflight` finds the marker present in 2 built files. 8
of 10 go red. In the first red leg, B's authored-cube query answers `400
INVALID_FIELD` and B's `meta` lists the request's definition. The
restore leg rebuilds, the marker is absent from all 6 built files, the
tree is clean, and all 10 cases pass again.
**Gates.** `node scripts/pm/dispatch-gates.mjs --repo
objectstack-ai/objectstack --commands` over this branch's 7 changed
paths gives 66 commands. All 66 exited 0, and the `--ran` reconciliation
reports 66/66 with 0 NOT MEASURED. `check:dual-build-cjs-loads` first
exited 3 (PREREQUISITE NOT MET: 8 packages had no `dist/`). It passed
after those packages were built, all from the turbo cache. `eslint
--no-inline-config` over the 6 changed TS files, all in the config's
`**/*.{ts,…}` population, gives 6 files, 0 errors and 0 warnings. The
config enables no type-aware linting (no `parserOptions.project` or
`projectService`) and no cross-file rules, so this diff cannot change
the verdict on an untouched file. The repo-wide `pnpm lint` is left to
CI.
## Merge note for the later lander (PR #20348, `analytics_cube.public`)
PR #20348 adds `assertCubePublic(queryInput.cube)` at the top of
`query()`. It reads `this.cubeRegistry.get(name)`. Here, `query()`'s
body lives in `queryIn(scope, …)`, so a textual merge lands that line in
`queryIn`, still reading the shared registry. It should read
`scope.getCube(name)`. Read from the shared registry, a dataset request
named like a hidden cube would be refused `404 CUBE_NOT_FOUND` with the
hidden-cube message, which is a new refusal on this door and an
existence signal for hidden names. The dogfood HIDDEN leg asserts 200,
so a merge that leaves it on the shared registry goes red there.
`getMeta`'s visibility filter correctly stays on the shared registry.
## Acceptance notes
- The `/analytics/query` and `/analytics/sql` doors still write the
shared registry at request time, before admission (`ensureCube`
inference and augmentation). This is reported in the dev report as a
separate finding. The `CubeScope` seam added here is the natural place
to scope them, but that changes the ad-hoc door's documented registry
source and overlaps PR #20348's `inferCubeFromQuery` edit, so it is not
done here.
- `strategies/native-sql-strategy.ts` (the join-allowlist comment near
the `getAllowedRelationships` refusal) still says `queryDataset`
registers the compiled dataset first. The invariant it states still
holds, through the request scope: the allowlist answers from the
compiled dataset and never falls through to the hook. The file is
outside this card's declared surface, so the wording is left for whoever
next touches it.
- The draft-preview branch still reads the pending seed rows before its
own admission check. This is a read, and nothing is returned on refusal.
It is unchanged.
- The one observable difference is stated in the changeset: a cube that
only a `queryDataset` call compiled is no longer listed by `getMeta()`
or queryable by name afterwards. No in-repo caller relies on that. A
search for runtime code querying a dataset name as a cube found none.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01TEah6PeJGjxJfbHaySJjLQ)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 2d91c9a commit 70ce802
7 files changed
Lines changed: 738 additions & 60 deletions
File tree
- .changeset
- packages
- qa/dogfood/test
- services/service-analytics/src
- __tests__
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
Lines changed: 266 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
Lines changed: 7 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
288 | 288 | | |
289 | 289 | | |
290 | 290 | | |
291 | | - | |
292 | | - | |
293 | | - | |
294 | | - | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
295 | 298 | | |
296 | 299 | | |
297 | 300 | | |
| |||
0 commit comments