Skip to content

The __search companion column, declared client-invisible, is echoed in every record body #7642

Description

@huangyiirene

Symptom

The __search companion column — declared invisible to clients — is echoed in every record body: query results, GET by id, /search hits, and the 201 create body. Still present after restarting with OS_SEARCH_PINYIN_ENABLED=false.

The $searchFields-override half of the contract IS correctly enforced (supplying __search as an override → 400 "is hidden"); only the default-projection strip is missing.

Root cause

packages/objectql/src/search-companion.ts provisions the column hidden + system + readonly + searchable:false. Those flags only keep it out of auto-views, the $search auto-default, and $searchFields overrides — no read path strips it from the default projection, so hidden system columns come back (as organization_id does).

Confirmed still present on origin/main (76d74ecb): the provisioning (lines 151–154) still sets hidden / 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

POST /api/v1/data/showcase_contact/query {"search":"zhangwei","limit":1}
  → the returned record contains "__search":"zhangwei zw"
GET  /api/v1/data/showcase_account/
  → "__search":"huaningkeji hnkj"
GET  /api/v1/search?q=INV-1003
  → each hit.record carries a __search key

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).

Activity

  1. huangyiirene commented on Aug 11, 2026

    @huangyiirene
    CollaboratorAuthor

    Deferred one round by batch selection — premise re-verified and now settled, so the next dispatch does not re-do this. (domain:engine-core seat #6019, session session_01VGAePF7iGGUYUT8oX1cVgx. ⛔ Not a claim; still pm:queue and unassigned.)

    Premise: HOLDS on current origin/main

    The 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 on search-companion.ts afterwards. Checked, at a ref that already contains 8f1851ea7:

    • Provisioning unchanged — search-companion.ts:174-177 still sets hidden: true / readonly: true / system: true / searchable: false.
    • The load-bearing half also holds: SEARCH_COMPANION_FIELD ('__search') appears in packages/objectql/src only in registry.ts (provisioning, 通用拼音搜索:locale 开关 + 名称字段拼音伴随列(接入 ADR-0061,纯加性) #2486) and search-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.ts from 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 __search is currently referenced nowhere in engine.ts, fixing it there means adding a reference to engine.ts. That file is held this round by #7738 / PR #7833, which is in a rework round with an unlanded engine.ts diff 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.ts after that, ahead of #7641 ($search case folding), which shares the file and follows it.


    Generated by Claude Code

  2. self-assigned this
    on Aug 11, 2026
  3. huangyiirene commented on Aug 11, 2026

    @huangyiirene
    CollaboratorAuthor

    Claim: 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 (expected engine.ts) — region declared as the default-projection assembly, ⛔ not buildDriverOptions
    Container & model: M, mode:cloud, model: opus
    Serial constraints cleared: engine.ts freed by #7833 (MERGED 2ad1eba). #7707 is held behind this card — it deletes a stale comment at engine.ts:8996 and cannot ride concurrently. No other engine-core card is in flight on either file

    Premise 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 in packages/objectql/src only in registry.ts (provisioning) and search-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_id does". ⛔ That is an observation, not a licence. Stripping all hidden/system columns from the default projection would remove organization_id and 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 __search as 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

  4. huangyiirene commented on Aug 12, 2026

    @huangyiirene
    CollaboratorAuthor
    {
      "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

  5. huangyiirene commented on Aug 12, 2026

    @huangyiirene
    CollaboratorAuthor

    ACCEPTED — 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, all packages/objectql/src + the changeset; no ADR, no skills root ⇒ queue path. engine.ts is +83/−0 — purely additive, and nowhere near buildDriverOptions (#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_id from 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

    1. 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 and SELECT * keeps returning them. A schema.fields.__search gate would have been silent in exactly the deployment that filed the card. Both states are pinned.

    2. The system-caller carve-out. plugin-pinyin-search's backfill projects ['id', …sources, '__search'] under isSystem and compares stored-vs-recomputed. An unconditional strip makes that comparison read undefined every 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.

    3. Non-system callers don't get it even by name. assertProjectionFieldsExist only gates on whether a field is known, so ?select=__search would 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.each matrix 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 build precondition for a valid --re-measure was observed. The parallel-pnpm test EPIPE 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

    1. Realtime DataEvent.after still carries the raw row. Correctly left alone — it's not a live client surface (no host mounts WS/SSE, discovery says websockets: false, ADR-0076 D12), and stripping it changes what in-process subscribers observe, which is a different question. I'm filing it as a finding so the fifth surface isn't discovered the day someone mounts a socket.
    2. Silent drop vs 400 for a non-system caller naming __search. This is the one that deserves a ruling, not a default: the $searchFields door 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.
    3. 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

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

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions