Repository navigation
The __search companion column, declared client-invisible, is echoed in every record body #7642
Description
Activity
- addedbugSomething isn't workingSomething isn't working
on Aug 11, 2026 huangyiirene commented
on Aug 11, 2026 CollaboratorAuthorMore actionsDeferred one round by batch selection — premise re-verified and now settled, so the next dispatch does not re-do this. (
domain:engine-coreseat #6019, sessionsession_01VGAePF7iGGUYUT8oX1cVgx. ⛔ Not a claim; stillpm:queueand unassigned.)Premise: HOLDS on current
origin/mainThe seat post carried an open caveat on this card — "last touched by PR #7591 (
8f1851ea7) ⇒ re-verify the premise at that ref or later", since this card was filed at 09:49Z and #7591 landed onsearch-companion.tsafterwards. Checked, at a ref that already contains8f1851ea7:- Provisioning unchanged —
search-companion.ts:174-177still setshidden: true/readonly: true/system: true/searchable: false. - The load-bearing half also holds:
SEARCH_COMPANION_FIELD('__search') appears inpackages/objectql/srconly inregistry.ts(provisioning, 通用拼音搜索:locale 开关 + 名称字段拼音伴随列(接入 ADR-0061,纯加性) #2486) andsearch-filter.ts(the recall OR-in). It appears in no read or projection path — so there is still nothing stripping it from the default projection, exactly as the card says. - Reverse-checked per the zero-hit discipline: a known-present control term returns 6 hits in
engine.tsfrom the same scan, so the empty result is a real absence and not a broken scan.
⇒ The caveat is discharged. #7591 did not move this mechanism, and the next PM to pick this up does not need to repeat the check.
One correction for the record: the ref this card cites as its confirmation point (
76d74ecb) does not resolve in a shallow working clone (51 commits), which reads exactly like a bogus sha. It is not one — it is the #7477/PR #7502 landing commit, simply outside the clone's depth. ⛔ Do not "fix" this card's Root cause section on that basis.Why it is deferred rather than dispatched now
The likely fix site is a read/default-projection path, and since
__searchis currently referenced nowhere inengine.ts, fixing it there means adding a reference toengine.ts. That file is held this round by #7738 / PR #7833, which is in a rework round with an unlandedengine.tsdiff in it.Same-file cards serialize strictly across rounds, and I hold no maintainer exemption to run the region-declared parallel variant. The regions would very likely be disjoint (#7833 owns
buildDriverOptions, ~engine.ts:2549-2560), but "very likely disjoint" is not the standard — and re-verifying a card that is already in rework because its last verification pass missed something is the worst moment to add a concurrent writer to the same file.Unblocks when PR #7833 lands. This card is first in line for
search-companion.tsafter that, ahead of #7641 ($searchcase folding), which shares the file and follows it.
Generated by Claude Code
- Provisioning unchanged —
huangyiirene commented
on Aug 11, 2026 CollaboratorAuthorMore actionsClaim: PM loop round 3 — unblocked by PR #7833 landing (
2ad1eba)
Session:session_01VGAePF7iGGUYUT8oX1cVgx
Branch:claude/issue-7642-search-companion-projection-strip
Worktree:objectstack-issue-7642
Domain:domain:engine-core
File surface:packages/objectql/src/search-companion.ts+ whichever read/default-projection path proves to be the real site (expectedengine.ts) — region declared as the default-projection assembly, ⛔ notbuildDriverOptions
Container & model: M,mode:cloud,model: opus
Serial constraints cleared:engine.tsfreed by #7833 (MERGED2ad1eba). #7707 is held behind this card — it deletes a stale comment atengine.ts:8996and cannot ride concurrently. No other engine-core card is in flight on either filePremise already verified by this seat at 19:5xZ — ⛔ do not re-verify, it is on the card above. Flags still at
search-companion.ts:174-177;SEARCH_COMPANION_FIELD('__search') appears inpackages/objectql/srconly inregistry.ts(provisioning) andsearch-filter.ts(recall OR-in), and in no read or projection path. Zero-hit reverse-checked against a known-present control.The fence that matters most on this card
The body notes hidden system columns come back generally, "as
organization_iddoes". ⛔ That is an observation, not a licence. Stripping all hidden/system columns from the default projection would removeorganization_idand friends from every response — a far larger, plausibly breaking change that no one has ruled on.⇒ Fix
__search. If measurement says the honest fix is a general hidden-column projection rule, report it as a fork and stop — that is a contract decision, and it goes to the decision box, not into this PR.⛔ Also do not touch the
$searchFields-override half: supplying__searchas an override already correctly 400s ("is hidden"). That half of the contract works; only the default-projection strip is missing.One measured detail worth carrying: the symptom survives a restart with
OS_SEARCH_PINYIN_ENABLED=false, so the fix has to cover already-provisioned columns, not just the provisioning path.
Generated by Claude Code
huangyiirene commented
on Aug 12, 2026 CollaboratorAuthorMore actions{ "issue": 7642, "premise_still_valid": true, "premise_evidence": "Re-confirmed on origin/main (1a19e9d) before the first edit, as a cheap re-read rather than a re-verification round: search-companion.ts:174-177 still provisions `hidden`/`readonly`/`system`/`searchable: false`, and SEARCH_COMPANION_FIELD appears in packages/objectql/src only in registry.ts (provisioning), search-filter.ts (recall OR-in) and the two barrel re-exports (core.ts, index.ts) — no read or projection path. The PM's measurement was correct in full; the two barrel lines are re-exports of the same symbol, not a third mechanism.", "pr": 7868, "pr_url": "https://github.com/objectstack-ai/objectstack/pull/7868", "pr_state": "draft", "branch": "claude/issue-7642-search-companion-projection-strip", "commit": "c5b8c7c", "fix_site": { "producer": "packages/objectql (engine) — the read/write path, NOT a consumer package", "sites": [ "packages/objectql/src/search-companion.ts:203 — isSearchCompanionRequested (new, pure)", "packages/objectql/src/search-companion.ts:246 — stripSearchCompanion (new, pure)", "packages/objectql/src/engine.ts:4760 — ObjectQL.stripSearchCompanionFromRead (new private seam, next to maskSecretFields)", "packages/objectql/src/engine.ts:6919 — find: caller's own `fields` captured before planFormulaProjection", "packages/objectql/src/engine.ts:6963 — find: strip, after afterFind + maskSecretFields", "packages/objectql/src/engine.ts:7086 — findOne: caller's own `fields` captured", "packages/objectql/src/engine.ts:7124 — findOne: strip", "packages/objectql/src/engine.ts:7727 — insert: strip on the caller-facing row, after afterInsert", "packages/objectql/src/engine.ts:8666 — update: strip before return" ] }, "surfaces": { "query_results": "covered — engine.find", "get_by_id": "covered — engine.findOne (protocol getData reads through it)", "search_hits": "covered — MetadataProtocol.searchAll builds a where/orderBy/limit query with NO `fields` and returns engine.find rows verbatim as hit.record, so the find strip reaches it. Pinned at the engine in the shape searchAll spells; the protocol call itself is not reachable from packages/objectql.", "create_201_body": "covered — engine.insert; createData returns its value verbatim as `record`", "also_covered_not_reported": [ "update 200 body — same column, same response shape; omitting it would make POST and PATCH on one object disagree about whether a client-invisible column is visible", "records nested by expand — expandRelatedRecords re-enters this.find" ], "not_covered": [ "delete response — deleteData returns {object, id, success}, no record body, nothing to strip", "realtime DataEvent `after` payload — engine.publishDataEvent carries the raw driver row, which still contains __search. NOT a live client surface today: no host mounts a WS/SSE surface and discovery reports `websockets: false` (service-realtime is an in-process bus, ADR-0076 D12). Flagged in open_questions rather than fixed, because it would mean stripping a payload server-side subscribers read." ] }, "scope_fence_respected": { "general_hidden_column_rule": "NOT taken. The strip names __search only; organization_id and every other hidden/system column are untouched. No fork to report — the honest fix for THIS defect is column-specific, so the general rule never became necessary.", "searchFields_override_half": "untouched", "buildDriverOptions": "untouched (PR #7833 region)", "engine_ts_8996_comment": "untouched (#7707 region)" }, "design_decisions": [ "The strip is NOT gated on the schema declaring the column. With OS_SEARCH_PINYIN_ENABLED=false the registry stops declaring the field, but the physical column and its values survive (ADR-0045 additive migrations) and SELECT * keeps returning them — a schema-gated strip would be silent in exactly the deployment that filed the card. The key on the row is the signal; both states are pinned.", "A SYSTEM caller that names __search in `fields` still gets it. plugin-pinyin-search's backfill/reconcile projects ['id', ...sources, '__search'] under {isSystem:true} and compares the stored blob to a recomputed one; an unconditional strip makes that comparison read undefined every pass, so the walk would rewrite every row of every object on every run.", "A NON-SYSTEM caller does not get it even by name. assertProjectionFieldsExist gates only on whether a field is KNOWN, and the companion is known once provisioned — so ?select=__search would otherwise be a documented spelling straight through the strip. This slightly exceeds a literal 'default projection' reading; it is still __search-only and does not touch the $searchFields half.", "The projection is read from the caller's ORIGINAL `fields`, captured before planFormulaProjection — that pass widens an explicit projection to every stored column when a formula is in play, companion included. Pinned as its own case.", "Mutation is in place, matching maskSecretFields; the driver contract forbids handing back live references into the backing store (MemoryDriver.find says so explicitly). Pinned by a no-write-through assertion." ], "tests": { "file": "packages/objectql/src/search-companion-read-projection-conformance.test.ts (new)", "placement_rationale": "A matrix over every record-returning door in both provisioning states, not a file testing the door that happened to be fixed — one contract with five independently-breakable doors is the shape that rots one door at a time. A door added later is one row.", "cases": 24, "reverse_check": "Reverted engine.ts alone and re-ran: 17 of 24 FAIL on the unfixed engine. The 7 that pass are the fixture guards (the stored row really carries the column), the no-write-through pins, and the 'system caller keeps it' case, which holds by construction pre-fix. The suite is not vacuous." }, "gates": { "pnpm check:query-options-erasure": "PASS (exit 0) — 67 unswept non-test sites in 17 files, none new; test surface 242 sites, at the ceiling, unchanged; baseline key set verified against 1a19e9d", "pnpm check:type-check-debt": "PASS (exit 0) — 33 ledger entries re-measured in 295s, 1789 raw errors, none above record. objectql held at its recorded 355. Ledger NOT raised: the first draft of the test file measured 358 (+3) and all three errors were fixed (EngineQueryOptions imported from @objectstack/spec/data rather than ./engine.js; `primaryKey` dropped from two field literals). pnpm build was run first, so the measurement is valid.", "pnpm test (whole repo, turbo --concurrency=1)": "PASS — 135/135 tasks, including the dogfood suite (91 files / 584 tests, real app boot)", "packages/objectql vitest (full)": "PASS — 187 files / 3312 tests, 0 failures", "adjacent suites": "PASS — @objectstack/rest 92 files/1471 tests; @objectstack/metadata-protocol 72/1066; @objectstack/plugin-pinyin-search 2/14", "pnpm --filter @objectstack/objectql typecheck": "PASS", "pnpm --filter @objectstack/objectql build": "PASS", "dispatch-gates matched": "PASS — check:adr-anchors, check:durability-log-level, check:engine-double-contract, check:stack-collection-maps, scripts/check-engine-split-ratio.mjs (metric only, prints 100% ratio, no threshold)", "eslint --no-inline-config on all changed files": "PASS — clean" }, "gate_caveats": [ "A first `pnpm test` at DEFAULT concurrency reported 7 failing packages (types, spec, metadata-core, driver-sql, mcp, downstream-contract, lint). Every one was a vitest worker EPIPE / unhandled 'error' event under parallelism, not an assertion; all 7 pass individually, and the --concurrency=1 full run above is green. Reporting it because the parallel run's output looks like real failures and the next reader should not re-diagnose it." ], "changeset": ".changeset/search-companion-default-projection-strip.md (@objectstack/objectql patch)", "files_changed": [ "packages/objectql/src/engine.ts (+83)", "packages/objectql/src/search-companion.ts (+57)", "packages/objectql/src/core.ts (+2, re-export)", "packages/objectql/src/index.ts (+2, re-export)", "packages/objectql/src/search-companion-read-projection-conformance.test.ts (new)", ".changeset/search-companion-default-projection-strip.md (new)" ], "public_api_added": ["stripSearchCompanion", "isSearchCompanionRequested"], "open_questions": [ "Realtime `data.record.created` / `data.record.updated` events carry the raw driver row as `after`, which still contains __search. Harmless today — no host mounts a WS/SSE surface and discovery advertises websockets:false — but the day one does, that is a fifth surface. Deliberately left alone: stripping it changes what in-process server-side subscribers observe, which is a different question from what a client is handed.", "A non-system caller naming __search in `select` now gets a silent drop rather than the value. The alternative is a 400 ('is hidden'), which would match how the $searchFields door already answers the same spelling. A drop was chosen because the projection door's existing contract for a field it will not return is silence, and a 400 here is a new refusal on a spelling that used to 200. Worth a ruling if the two doors should agree.", "The strip is column-specific by decree. If the platform ever wants 'hidden system columns are not client-visible' as a rule, the seam to generalise is ObjectQL.stripSearchCompanionFromRead — but that removes organization_id and its siblings from every payload and needs the decision box first." ], "blocked": false }
Generated by Claude Code
huangyiirene commented
on Aug 12, 2026 CollaboratorAuthorMore actionsACCEPTED — PR #7868 flipped ready and going to the queue.
Gate conclusions read from the jobs at head
c5b8c7c: ESLint success (00:44:26Z), TypeScript Type Check success (00:47:55Z), 25/25. Path face: 6 files, allpackages/objectql/src+ the changeset; no ADR, no skills root ⇒ queue path.engine.tsis +83/−0 — purely additive, and nowhere nearbuildDriverOptions(#7833's settled region) or:8996(#7707's). Both fences held.The fence I cared most about held, and for a better reason than I gave
I fenced hard against generalising to all hidden system columns, because that would strip
organization_idfrom every payload. You didn't just obey it — you reported that no fork was needed: the honest fix for this defect turned out to be column-specific on its own merits, so the general rule never became necessary. That is the right way to answer a fence: not "I was told not to," but "I checked, and it doesn't arise."Three judgements that are better than the card's
-
Not gating the strip on the schema declaring the column. The card's sharpest detail was that the symptom survives
OS_SEARCH_PINYIN_ENABLED=false, and you worked out why: with the switch off the registry stops declaring the field, but ADR-0045 migrations are additive, so the physical column and its values remain andSELECT *keeps returning them. Aschema.fields.__searchgate would have been silent in exactly the deployment that filed the card. Both states are pinned. -
The system-caller carve-out.
plugin-pinyin-search's backfill projects['id', …sources, '__search']underisSystemand compares stored-vs-recomputed. An unconditional strip makes that comparison readundefinedevery pass, so the walk would rewrite every row of every object on every run — write amplification traded for a low-risk disclosure fix. Finding that before shipping is the difference between a fix and an incident. -
Non-system callers don't get it even by name.
assertProjectionFieldsExistonly gates on whether a field is known, so?select=__searchwould have been a documented spelling straight through the strip. You flagged that this slightly exceeds a literal "default projection" reading rather than sliding it in — correct on both counts.
The test file is the shape I asked for and the reverse-check proves it
A
describe.eachmatrix over both provisioning states × every record-returning door, not a test for the door that happened to break. 17 of 24 fail on the reverted engine, and you explained the 7 that don't (fixture guards, no-write-through pins, and the system-keeps-it case, which holds pre-fix by construction). That is a non-vacuous suite with its own vacuity guards — the fixture assertion "the stored row DOES carry the column" is exactly the control that makes the other 23 mean something.Covering the update 200 body unprompted was right: it isn't one of the four reported surfaces, but leaving it out would have made POST and PATCH on one object disagree about whether a client-invisible column is visible.
Ledger discipline noted
The first draft measured TEST_DEBT 358 (+3) and you fixed all three rather than raising the entry — the same trap that cost #7738 a full round. And the
pnpm buildprecondition for a valid--re-measurewas observed. The parallel-pnpm testEPIPE caveat is a genuinely useful thing to have written down; it looks exactly like 7 real package failures and it isn't.Your three open questions — routing
- Realtime
DataEvent.afterstill carries the raw row. Correctly left alone — it's not a live client surface (no host mounts WS/SSE, discovery sayswebsockets: false, ADR-0076 D12), and stripping it changes what in-process subscribers observe, which is a different question. I'm filing it as afindingso the fifth surface isn't discovered the day someone mounts a socket. - Silent drop vs 400 for a non-system caller naming
__search. This is the one that deserves a ruling, not a default: the$searchFieldsdoor already answers that same spelling with a 400, and two doors disagreeing about one column is how the next reader gets confused. Going to the decision box — ⛔ nothing in this PR depends on it. - The generalisation seam (
ObjectQL.stripSearchCompanionFromRead) is named and left. Right — that's a contract decision.
Landing now. #7707 unblocks on merge.
Generated by Claude Code
-
Symptom
The
__searchcompanion column — declared invisible to clients — is echoed in every record body: query results, GET by id,/searchhits, and the 201 create body. Still present after restarting withOS_SEARCH_PINYIN_ENABLED=false.The
$searchFields-override half of the contract IS correctly enforced (supplying__searchas an override → 400 "is hidden"); only the default-projection strip is missing.Root cause
packages/objectql/src/search-companion.tsprovisions the columnhidden+system+readonly+searchable:false. Those flags only keep it out of auto-views, the$searchauto-default, and$searchFieldsoverrides — no read path strips it from the default projection, so hidden system columns come back (asorganization_iddoes).Confirmed still present on
origin/main(76d74ecb): the provisioning (lines 151–154) still setshidden/readonly/system/searchable:false, with no default-projection strip on any read path.Impact
Disclosure risk is low — the value derives from the display field only, and secret/FLS-restricted sources are excluded. But the declared client-invisibility contract is broken and every payload carries a private index column.
Reproduction
The 201 body of a create carries it too. Still echoed after restarting with
OS_SEARCH_PINYIN_ENABLED=false.Source
Extracted from the QA run #7629 (framework 92f26f7, console 6314e87f).