Uh oh!
There was an error while loading. Please reload this page.
fix(objectql): strip the hidden __search companion from every record body (#7642) - #7868
Conversation
…d body (#7642) The `__search` search-normalization companion (#2486) is declared invisible to clients — `hidden` + `readonly` + `system` + `searchable: false` — and every one of those flags does something real: the column stays out of auto-views, out of the `$search` auto-default, and a `$searchFields` override naming it is refused with a 400 ("is hidden"). None of them is a PROJECTION rule. A query that names no `fields` reaches the driver with `ast.fields` undefined, drivers answer that with `SELECT *`, and the column rode back in the four record bodies QA measured (#7629): query results, GET by id, `/search` hits, and the 201 create body. The strip runs at the engine, the producer all four surfaces share — `/search` hits are `engine.find` rows verbatim, the create body is `engine.insert`'s return verbatim, so fixing consumers one at a time would have left three of the four broken. Covered: `find`, `findOne`, the nested records `expand` produces, the create response and the update response. The update response is not one of the four reported surfaces but is the same column in the same response shape; leaving it out would make POST and PATCH on one object disagree about whether a client-invisible column is visible. A predicate update resolves to a count and is unaffected. Two shaping details, both from the report: - Not gated on the schema declaring the column. The symptom survived a restart with `OS_SEARCH_PINYIN_ENABLED=false`: with the switch off the registry stops DECLARING the field, but the physical column and its values remain (ADR-0045 migrations are additive) and `SELECT *` keeps returning them. A strip that asked `schema.fields.__search` first would be silent in exactly the deployment that filed the bug. - One caller keeps its read. `plugin-pinyin-search`'s backfill projects `['id', ...sources, '__search']` under a system context and compares the stored blob against a recomputed one; stripping it unconditionally would make the walk rewrite every row of every object on every pass. A SYSTEM caller that names the column still gets it — a non-system caller does not, even by name, since `select` only gates on whether a field is KNOWN and `?select=__search` would otherwise be a documented way straight through the strip. Scope is this one column. Hidden system columns do come back generally (`organization_id` and its siblings), but they are load-bearing in client payloads today; removing them is a contract decision, not a defect fix. The new suite is a MATRIX over every record-returning door rather than a test for the door that was fixed, in both provisioning states — one contract, five places that can break it independently is the shape that rots one door at a time. Reverse-checked: 17 of its 24 cases fail on the unfixed engine.
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 1 package(s): 15 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also reference the affected code. These are read-only:
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#7642
What was broken
The
__searchsearch-normalization companion (#2486) is declared invisible to clients —hidden+readonly+system+searchable: false— and every one of those flags does something real: the column stays out of auto-views, out of the$searchauto-default, and a$searchFieldsoverride naming it is refused with a 400 ("is hidden"). None of them is a PROJECTION rule. A query that names nofieldsreaches the driver withast.fieldsundefined, drivers answer that withSELECT *, and the column rode back in the four record bodies the QA run measured (#7629): query results,GETby id,/searchhits, and the 201 create body.Premise re-confirmed on
origin/main(1a19e9d) before the first edit:search-companion.ts:174-177still provisions the flags, andSEARCH_COMPANION_FIELDstill appears inpackages/objectql/srconly inregistry.ts(provisioning),search-filter.ts(the recall OR-in) and the two barrel re-exports — no read or projection path.The fix site
packages/objectql/src/engine.ts— the default-projection read path, at the same seam asmaskSecretFields. The engine is the producer all four surfaces share:/searchhits areengine.findrows verbatim (MetadataProtocol.searchAll→hit.record), the 201 body isengine.insert's return verbatim (createData→record),GETby id isfindOne(getData). Fixing consumers one at a time is how three of the four would have stayed broken.findengine.ts:6963GETby idfindOneengine.ts:7124/searchhitssearchAll→engine.findfindinsertengine.ts:7727updateengine.ts:8666— see belowexpandrecordsexpandRelatedRecords→this.findfinddeleteDatareturns{object, id, success}The update response is not one of the four reported surfaces. It is the same column in the same response shape, and leaving it out would make POST and PATCH on one object disagree about whether a client-invisible column is visible, so it is covered. A predicate update resolves to an affected-row count (#4639) and is unaffected.
Two details the fix is shaped around
It is not gated on the schema declaring the column. The report's sharpest detail is that the symptom survived a restart with
OS_SEARCH_PINYIN_ENABLED=false. That is not a stale process: with the switch off the registry stops declaring the field, but the physical column and its values remain (ADR-0045 migrations are additive) andSELECT *keeps returning them. A strip that askedschema.fields.__searchfirst would be silent in exactly the deployment that filed the bug — so the key on the row is the signal. Both states are pinned.One caller keeps its read.
plugin-pinyin-search's backfill/reconcile walk projects['id', ...sources, '__search']under{ isSystem: true }and compares the stored blob against a recomputed one; stripping that unconditionally would make it rewrite every row of every object on every pass — write amplification traded for a disclosure the issue rates low. So a system caller that names the column infieldsstill gets it. A non-system caller does not, even by name:assertProjectionFieldsExistonly gates on whether a field is known, so?select=__searchwould otherwise be a documented spelling straight through the strip. The projection is read from the caller's originalfields, captured beforeplanFormulaProjection— that pass widens an explicit projection to every stored column when a formula is in play, companion included.Scope fence
This one column only. The issue observes that hidden system columns come back generally, "as
organization_iddoes". That is an observation, not a licence: stripping every hidden/system column from the default projection would removeorganization_idand its siblings from every response, which is a contract decision and not a defect fix. Nothing here touches the$searchFields-override half either — that half already answers 400 correctly.Tests
packages/objectql/src/search-companion-read-projection-conformance.test.ts— a matrix, not a test for the door that happened to be fixed. One contract with five independently-breakable doors is the shape that rots one door at a time, so every door runs the same assertion from the same table, in both provisioning states (declared / flag-off-with-stored-values). Also pinned: the fixture is non-vacuous (the stored row really carries the column), the strip does not write through to the backing store, the formula-widened projection still strips, and both halves of the system-caller exemption.Reverse-checked by reverting
engine.tsalone: 17 of the 24 cases fail on the unfixed engine (the 7 that pass are the fixture guards, the no-write-through pins, and the "system caller keeps it" case, which holds by construction pre-fix).Gates
pnpm check:query-options-erasurepnpm check:type-check-debtpnpm test(whole repo,--concurrency=1)packages/objectqlvitestpnpm --filter @objectstack/{rest,metadata-protocol,plugin-pinyin-search} testpnpm --filter @objectstack/objectql typecheck+buildcheck:adr-anchors,check:durability-log-level,check:engine-double-contract,check:stack-collection-maps,check-engine-split-ratio(viascripts/pm/dispatch-gates.mjs)eslint --no-inline-configon all changed filesA first
pnpm testat default concurrency reported 7 package failures; every one was a vitest workerEPIPE/crash under parallelism, and all 7 pass individually — the--concurrency=1run above is the real number.Region fence
buildDriverOptions(~engine.ts:2549-2560, PR #7833) is untouched, and no comment atengine.ts:8996was edited (#7707's region).Generated by Claude Code