Skip to content

fix(service-analytics): on SQLite, the text operators compare the whole stored value and comparand, not only up to their first U+0000 (#20025) - #20067

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-20025-analytics-sqlite-text-nul
Sep 25, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-20025-analytics-sqlite-text-nul

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #20025

Clause-②: no

What this changes

service-analytics compiles its own SQL for the text operators, in text-match-sql.ts textMatchPredicateSql, and three faces call it with dialect: 'sqlite':

  • compileScopedFilterToSql (read-scope-sql.ts), the read scope, reached directly and through both strategies' scope merge;
  • NativeSQLStrategy.buildFilterClause, the executed where;
  • ObjectQLStrategy.buildFilterClauseSql, the /analytics/sql echo, which its caller runs on the same database.

Its sqlite arm emitted GLOB for every shape. SQLite's glob() reads its pattern AND the stored value as C strings, so each is cut at its first U+0000. driver-sql left that construct in #19999 (the comparand's cut) and #20024 (the stored value's cut); this package re-emits the driver's construct table rather than importing it, and its copy had kept GLOB.

The sqlite arm now mirrors driver-sql's textMatchPredicate / sqliteLengthAwareTextMatch on main, cell for cell:

shape SQLite predicate
contains ($contains, $notContains, $icontains) instr(col, ?) > 0
ends ($endsWith), non-empty comparand coalesce(substr(CAST(col AS BLOB), -length(CAST(? AS BLOB))), CAST(col AS BLOB)) = CAST(? AS BLOB)
ends, empty comparand instr(col, ?) > 0
starts ($startsWith), comparand without U+0000 col GLOB ?, unchanged byte for byte
starts, comparand holding U+0000 instr(col, ?) = 1
  • This is the wider split the card's suggested shape predates. glob() also cuts the stored value, so every contains / ends comparand moves, not only a comparand holding U+0000. Only $startsWith without U+0000 stays on GLOB, because neither cut can change that answer and it is the one shape an index serves.
  • The fold is still lower() on both sides, ASCII-only ($regex on driver-sql is not a regex — it compiles to a substring LIKE, so it both over-matches and silently matches nothing #4706 Q1 = A). The four case-exact operators stay case-sensitive (Q2 = A); instr() and the BLOB comparison are byte-wise.
  • The negation is NOT (…), which is NULL for a NULL value, as NOT GLOB was. So the NULL-safe $notContains wrapper and the $not totalisation compose unchanged (非否定路径上的 $ne / $nin / $notContains:driver-sql 排除 NULL 行,driver-memory / formula 返回它们(#5146 只裁定了 $not) #5298).
  • Every comparand refusal, and the non-text-column constant, still runs before the text arm. Neither door is touched.
  • The suffix arm binds its comparand twice, in placeholder order, through the existing TextMatchBind. The LIKE arms already bind two values, so none of the three call sites changes.
  • Nothing is escaped on the new constructs. instr() and the BLOB suffix have no pattern language, so *, ? and [ bind as written.
  • text-match-sql.ts's header now describes the sqlite arm as it is, and names the new parity suite beside text-operator-case-exactness.test.ts.

Measured before the fix: the face table

Executed, not read, at 980bc05e5b. That is main after PR #20046 landed, and it moved nothing in text-match-sql.ts since the dispatch base 2274894cc4. It ran on better-sqlite3 (SQLite 3.53.4) and sql.js (3.49.1), each through its driver's own execute(). Every cell was compared with @objectstack/formula's matchesFilterCondition.

The grid: stored values with U+0000 at the start, middle and end, without it, '' and NULL; comparands with and without U+0000; each leaf bare and under $not. All five faces answered alike, and so did both engines: the direct read scope, the read scope through NativeSQLStrategy and through the echo, the native where, and the echo where.

operator comparand without U+0000 comparand holding U+0000
$contains narrows → matches JS widens → matches JS
$notContains widens → matches JS narrows → matches JS
$startsWith matches JS (unchanged, still GLOB) widens → matches JS
$endsWith widens and narrows → matches JS widens → matches JS
$icontains narrows → matches JS widens → matches JS
each of the above under $not the opposite direction → matches JS the opposite direction → matches JS
  • In this PR's pin grid (text-match-sqlite-nul.test.ts, 128 cells), every face on both engines answered 88 cells differently from JS at base, and answers 0 at head.
  • The ObjectQL execute face goes through the engine to driver-sql, which is already fixed on main. It is the control, and it answered all 128 like JS at base and at head.
  • The unknown arm on SQLite emits LIKE, and SQLite's LIKE cuts at U+0000 too. That was measured on both engines: 'a' + U+0000 + 'b' is not LIKE '%b%', and 'plain' is LIKE '%' + U+0000 + '%'. The arm is untouched. It is the residue for dialects nothing answered for, PostgreSQL-like ones among them, and PostgreSQL has no instr(). Choosing a construct there would be a dialect guess. It is reported to the seat as a finding and noted in the header.
  • $like / $ilike are refused on every analytics face, before this arm: READ_SCOPE_COMPILE_FAILED on the read scope, INVALID_FILTER / 400 on the where. This package compiles no raw-pattern operator, so there is nothing to swap.

Controllability (triage's point 1)

  • Comparand side: no caller-controlled producer measured. The RLS compile path (RLSCompiler.compileFilter → compileCelToFilter) resolves a string method's argument from one of two places:

    • a literal in the admin-authored policy;
    • a string attribute of current_user (id, organization id, email).

    The ids are system-generated. The email is refused at every auth entry measured when it holds U+0000, and it is read-only on the user object. Membership sets are arrays, which the string methods refuse. plugin-sharing emits id / owner membership scopes, not text operators. Sharing-rule criteria are admin-authored and evaluated through engine.find, the driver face that is already fixed.

  • Stored-value side: a caller-controlled producer exists. It is any persona that holds create or edit on an object with a text field. The engine's write path stores U+0000 unchanged under a non-system context (executed). Reaching a read scope also needs a deployment-authored using predicate with a string method over such a field. No shipped example authors one.

Tests (at d000746b1d)

  • New src/__tests__/text-match-sqlite-nul.test.ts, 29 tests. It covers:

    • the 128-cell grid on every face, on better-sqlite3 and sql.js, each through its driver's execute(). That is the transport plugin.ts uses, and a bare sql.js bind would cut a comparand holding U+0000 before any construct saw it;
    • each cell equal to the JS answer AND to driver-sql's own rows on the same engine. That is the parity pin that keeps the two construct tables from drifting;
    • literal pins for the headline cells;
    • the compiled constructs per shape, including $startsWith without U+0000 emitting GLOB and its escaped pattern byte for byte on all three compilers.

    It was committed first, red at base (ced01cbad7): 14 failed and 15 passed; each face on each engine had 88 of 128 cells differ; driver-sql had 0.

  • Respelled pins, never loosened. Four existing files asserted GLOB for shapes that no longer compile to it: text-operator-case-exactness.test.ts, icontains-dialect-sql.test.ts, sql-dialect-vocabulary.test.ts and text-operator-non-text-column.test.ts.

    • Each now names the exact per-operator construct.
    • The read-scope and echo loops check a per-operator regex plus "not LIKE", where they used one /GLOB/.
    • The $endsWith / $startsWith compiled-text rows gained SQL assertions they did not have.
  • pnpm --filter @objectstack/service-analytics test: 121 files, 2718 tests passed. typecheck (tsc --noEmit, whose program lists the touched test files) exit 0.

Ablations: each negative pin, one shape at a time, through scripts/ablation-replace.mjs

The subject is src/text-match-sql.ts, imported relatively by every face, so vitest reads source and no dist/ is involved. Every leg landed on disk (the anchor count went from 1 to 0 and the blob changed) and was restored with its blob equal to HEAD (e913134711cb) and git diff HEAD empty.

leg mutation red
A1 contains back on GLOB 21 tests; 60/128 cells per face and engine
A2 ends back on GLOB 15 tests; 22/128 cells
A3 starts holding U+0000 back on GLOB 11 tests; 6/128 cells
A4 starts without U+0000 moved off GLOB 4 compiled-text tests (the byte-for-byte pins); rows unchanged, as expected
A5 empty ends sent to the suffix construct 11 tests; 2/128 cells ('' bare and under $not)
A6 coalesce() removed from the suffix 14 tests; 12/128 cells ('' under $not)

The first A6 attempt was a no-op. Its replacement text already occurred inside its own anchor, so the tool refused the leg and ran nothing. It was re-run with a non-overlapping anchor (the row above).

EXPLAIN QUERY PLAN over an indexed TEXT column, on both engines: $contains / $endsWith scanned under GLOB and still scan, and $startsWith without U+0000 keeps its SEARCH … USING INDEX.

Gates (at d000746b1d)

  • The dispatch-gates derivation: node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, run from the real diff. It gave 60 families, identical to the dispatch-time list.
    • 58 exit 0.
    • 2 are NOT MEASURED: check:dual-build-cjs-loads and check:type-check-debt both exited 3 with PREREQUISITE NOT MET, because they read the whole repo's built closure and this worktree built only the package's own. CI builds it first.
    • --ran reconciliation: 60 derived, 58 run, 2 NOT MEASURED, 0 unrun.
  • GITHUB_TOKEN=… node scripts/check-issue-citations.mjs: every citation this change adds resolves.
  • Lint, a declared narrowing. ESLint ran on the 6 changed TS files (--no-inline-config --format json): 6 files, 0 errors, 0 warnings.
    • The population is the config's packages/**/*.{ts,…} block; none of the six is in NEVER_LINTED.
    • Type-aware linting is not enabled: --print-config shows parserOptions without project. So this diff cannot move a verdict on an untouched file.
    • The full pnpm lint is CI's.

Acceptance notes

  • Surface. The claim named text-operator-case-exactness.test.ts and new test files. Three more existing test files carried GLOB pins for the moved shapes and are respelled here (listed above). No call site, refusal door, driver, spec or formula file is touched.
  • Stale prose, no assertion reads it. These comments still say the SQLite arm is GLOB for every shape: like-pattern.ts (the $icontains note), read-scope-sql.ts (the textMatch and $icontains docblocks), strategies/native-sql-strategy.ts, strategies/objectql-strategy.ts and strategies/types.ts. They ride the next PR that touches those files. Carrier: none.
  • Observation: $icontains: ''. It is not a U+0000 cell, and this change does not move it.
    • These compilers, and AnalyticsService.query in-process, answer every non-NULL row for it, as they did under GLOB '**'.
    • The spec's FILTER_TEXT_CASES declares it refused (INVALID_FILTER), and driver-sql refuses it.
    • Reachability through the REST door is not measured. It is reported to the seat with its evidence.
  • main moved during this run only by 3557f85fa5 (a driver-turso README), which touches nothing here. No merge was needed.

Generated by Claude Code

…ery face (red at base)

Records the #20025 measurement on the branch before the fix: the five faces
that call `textMatchPredicateSql` with `dialect: 'sqlite'` are run on
better-sqlite3 and sql.js (through each driver's own `execute()`) over a
grid of stored values and comparands with and without U+0000, `''` and
NULL, bare and under `$not`. At this commit every face answers 88 of 128
cells differently from the JavaScript rows, while driver-sql answers all
128 alike. The fix follows in the next commit.

Claude-Session: https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF
Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Sep 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json b76aad5f6fac71cbbcd4ea6bfdf21a59e2fd4d56 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 2d3d274488dc58e2ddb7d9a87a513216ff61de26 — the merge of head d000746b1d4ac930a621ace8581c518ede1137c2 into base b76aad5f6fac71cbbcd4ea6bfdf21a59e2fd4d56, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 2d3d274488dc58e2ddb7d9a87a513216ff61de26 && git checkout 2d3d274488dc58e2ddb7d9a87a513216ff61de26
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin b76aad5f6fac71cbbcd4ea6bfdf21a59e2fd4d56 d000746b1d4ac930a621ace8581c518ede1137c2 && git checkout -B drift-repro b76aad5f6fac71cbbcd4ea6bfdf21a59e2fd4d56 && git merge --no-ff d000746b1d4ac930a621ace8581c518ede1137c2

node scripts/docs-audit/affected-docs.mjs --json b76aad5f6fac71cbbcd4ea6bfdf21a59e2fd4d56

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

1 participant