Data query: an unknown field inside `where` / `$filter` answers 200/0 instead of `400 INVALID_FIELD` — the bare-key door disagrees (#4134's uncovered sibling)

[huangyiirene]

Symptom

POST /api/v1/data/showcase_invoice/query with {"where":{"not_a_field":"x"}} answers:

200 { "records": [], "total": 0 }

— no code, no mention of the unknown name. Identically for the $filter door (GET ...?$filter=...) and for the filter-AST door (POST {filter: [...]}).

Expected: 400 INVALID_FIELD naming the field, the same envelope the sibling door already produces.

Same-run control on the same object, same field name: GET /api/v1/data/showcase_invoice?not_a_field=x → 400 INVALID_FIELD.

So one endpoint family answers two different verdicts for one mistake, chosen by which door the caller used — and the losing answer is "indistinguishable from no data", which is exactly what #4134 closed for the bare-key door: an empty result set that a client cannot tell apart from a filter that legitimately matched nothing.

Root cause

The #4134 read-path field-existence gate (assertQueryParamsAreFields, backed by resolveQueryFields, in packages/metadata-protocol/src/protocol.ts) covers only the implicit filters findData derives from leftover query parameters. The explicit filter axes — a where object, a $filter string, a filter AST — reach the driver without passing that gate, so an unknown key is lowered into a field-equality predicate that can only match zero rows.

Relationship to #4134 (closed/completed, fixed by PR #4169): this is not a regression of it — the bare-key control still answers 400 INVALID_FIELD in this run. It is the sibling door #4134's fix did not reach, and the gate it introduced (resolveQueryFields, explicitly documented as "ONE resolution shared by all four read axes") is the natural place for the fix to land.

Reproduction

1. Boot showcase on a fresh isolated file DB (SqlDriver / better-sqlite3); authenticate as admin@objectos.ai.
2. POST /api/v1/data/showcase_invoice/query body {"where":{"not_a_field":"x"}} → 200 {"records":[],"total":0}, no code.
3. Repeat through the $filter door and the AST (filter) door — same 200/0.
4. Control, same run and same object: GET /api/v1/data/showcase_invoice?not_a_field=x → 400 INVALID_FIELD.

Source

Extracted from the QA run #7463 (framework a86db17).

[os-zhuang]

Claimed — domain:metadata seat.

• Session: session_01NxE7c6qf7Bi9ZQ7HtYrNUj
• Branch: claude/issue-7534-explicit-filter-field-gate
• Worktree: ../objectstack-7534 (dedicated; the shared primary checkout is not edited)
• File surface: packages/metadata-protocol/src/protocol.ts — the explicit filter axes' path into resolveQueryFields / assertQueryParamsAreFields, plus new pins.

⚠️ Serialization constraint, named: #7532 is dispatched in parallel on the projection axis (assertProjectionFieldsExist) of this same gate family, in the same file. Both dispatches carry an explicit stop: neither dev may restructure the shared resolveQueryFields resolution — an additive call into it from their own axis is what each card expects. If either concludes the shared helper must change shape, they stop and report rather than doing it in parallel.

The dispatch also requires re-verifying at the branch point that the bare-key control still answers 400 INVALID_FIELD; if it does not, this stops being #4134's uncovered sibling and becomes a regression card, which is a different piece of work.

---

Generated by Claude Code

[claude]

{
  "issue": 7534,
  "status": "done",
  "branch": "claude/issue-7534-explicit-filter-field-gate",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/7587",
  "premise_still_valid": true,
  "summary": "Confirmed the premise by measurement at the branch point (9051802) BEFORE implementing: the bare-key control answered 400 INVALID_FIELD while the where-object, $filter-string and filter-AST doors each answered 200/total=0 on the same object and the same field name. So this is #4134's uncovered sibling, not a regression of it. Fix is a new private `assertFilterFieldsExist` in packages/metadata-protocol/src/protocol.ts that calls the EXISTING `resolveQueryFields` on the normalized `where`; one call covers all three doors because the four filter spellings fold to one slot at the #3795 fold and a filter AST is lowered by `parseFilterAST` before the gate runs, so the gate reads the same FilterCondition the driver reads. Per the #7532 serialization constraint I did NOT reshape the shared helper: `resolveQueryFields`, `assertQueryParamsAreFields` and `assertProjectionFieldsExist` are byte-identical to main (verified with `git diff main -- ` on the hunk boundaries; the whole diff is +150 lines, all additive). Ordering deliberately unmoved: after the #4134 param gate (existing precedence holds) and before the #4164 merge (so the rejection can name the axis the caller used). NOTE for the PM, one downstream behaviour change no test pinned either way: in packages/rest/src/import-runner.ts, `findExisting` with a matchField naming a nonexistent field previously returned 'none' and silently degraded an upsert into an INSERT; it now yields a per-row INVALID_FIELD failure (contained by the row loop's own try/catch). The sibling `resolveRef` probe already wrapped its speculative field probes in `catch { /* field absent on target object */ }` and is unaffected.",
  "tests": "BRANCH-POINT MEASUREMENT (9051802, before any fix), protocol.findData against a real ObjectQL engine + registry:\n  PROBE CONTROL bare-key    => REJECTED status=400 code=INVALID_FIELD field=not_a_field\n  PROBE DOOR where-object   => RESOLVED total=0 records=0\n  PROBE DOOR $filter-string => RESOLVED total=0 records=0\n  PROBE DOOR filter-AST     => RESOLVED total=0 records=0\n  PROBE DOOR filter-AST-single => RESOLVED total=0 records=0\n  PROBE DOOR where-nested-$and => RESOLVED total=0 records=0\n  PROBE SANITY real field where => RESOLVED total=2 records=2\nControl holds at the branch point => card keeps its stated character.\n\nAFTER THE FIX, same probe: all six unknown-field doors REJECTED 400 INVALID_FIELD; SANITY still RESOLVED total=2.\n\nTARGETED GATES (all foreground, all on branch HEAD d01ae81):\n$ npx vitest run src/protocol-explicit-filter-field-gate.test.ts src/protocol-unknown-query-param.test.ts  # packages/objectql\n   Test Files  2 passed (2)\n        Tests  38 passed (38)\n$ npx vitest run   # packages/metadata-protocol (the changed package)\n   Test Files  71 passed (71)\n        Tests  1051 passed (1051)\n$ npx vitest run   # packages/objectql (where the new suite lives)\n   Test Files  179 passed (179)\n        Tests  3175 passed (3175)\nCONSUMPTION RADIUS (grepped every findData caller repo-wide, then ran the suites, since grep misses fragment- and regex-shaped pins):\n   @objectstack/rest      1341 passed (82 files)\n   @objectstack/runtime   1976 passed (122 files)\n   @objectstack/client     279 passed (21 files)\n$ npx turbo run typecheck --filter=@objectstack/metadata-protocol --filter=@objectstack/objectql\n   Tasks: 15 successful, 15 total\n$ npx eslint <changed files> --no-inline-config  => clean\n\nREVERSE VERIFICATION (predictions written BEFORE running; revert done by neutralising the gate BODY, because removing the call site fails the build with TS6133 and would have run the suite against a stale dist -- my first revert attempt did exactly that and its all-green result was discarded as meaningless):\n   11 tests predicted RED went RED: the 3 door tests, AST single/and-group spellings, nested $and/$or/$not, operator bag on an unknown field, four-door agreement, param wire-spelling, zero-row message + typo suggestion, first-unknown-plus-rest, dotted path on an UNKNOWN head.\n   11 tests labelled GUARD stayed green in BOTH directions: baseline, every-door-applies-a-real-filter, the #4134 bare-key control, honest 200/total=0 for a real field matching nothing, registry-injected system fields, dotted path on a REAL head, nested-relation value not descended into, unknown object stays 404, unrunnable filter stays INVALID_FILTER, #4164 composition, param-gate-reports-first precedence.\n\nMISSED PREDICTION (disclosed): one test I labelled GUARD went RED on revert -- the dotted-path case. Cause was a mislabel, not a bad test: I had bundled a fix-dependent assertion (not_a_field.name => 400) together with a genuinely both-directions-green one (owner_id.name passes through). Split into two correctly-labelled tests; re-run confirms 11 RED / 11 green.\n\nFIXTURE BUG caught by a failed GREEN prediction: 'every explicit door applies a REAL filter' failed on first run. The door table built its real-field variant with JSON.stringify(...).replace(/\"x\"/g, ...), which silently misses on the $filter door because that door nests JSON inside JSON and escapes its quotes. Per the working rule I suspected the fixture rather than the gate; the table now takes the value as a parameter and the reason is written into the file.\n\nANTI-VACUOUS: positive identity is pinned first -- every door is shown to APPLY a real filter and return the two expected rows by id before any refusal is asserted. The four-door agreement test builds an explicit verdict list and asserts toEqual on all four strings, so a door answering something NEW also fails. Both loop-based assertions carry a toHaveLength scale guard.\n\nGATE STATUS: CI on PR #7587 is in_progress -- convergence is the reviewer's read, not mine. A full-monorepo `turbo run test` was started locally and I KILLED it rather than block this report; an earlier full-parallel run had reported failures in types/spec/metadata-core/metadata-fs, but each of those passes standalone AND under turbo when run alone (types: 9 files / 230 tests green), so that was vitest resource contention, not this change -- and per this repo's own rule (os-dev.md: targeted gates locally, the full farm is CI's job) the full run was not owed.\n\nNOT MEASURED: no live showcase boot over HTTP. The issue's repro boots showcase on a fresh better-sqlite3 file DB; I measured at the findData ingress with a real ObjectQL engine + registry -- the layer the gate lives at and the layer all four doors converge on -- so I have NOT personally observed 400 INVALID_FIELD on the wire against showcase_invoice. The @objectstack/rest suite covers the route -> mapDataError -> envelope leg. Also only the in-suite stub driver was exercised, and $nor / unrecognised $-combinators are documented as an intentional hole (a hole, not a false rejection) but not tested as one.",
  "open_questions": [
    {
      "question": "import-runner `findExisting`: a matchField naming a nonexistent field now fails the row with INVALID_FIELD instead of silently degrading an upsert into an insert. Keep, or exempt the import path?",
      "options": [
        "A. Keep it \u2014 a matchField that names no field cannot ever match, so the silent upsert->insert degradation was a data-correctness bug of the same family as this card.",
        "B. Exempt the import path by catching INVALID_FIELD in findExisting and returning 'none', preserving today's behaviour exactly."
      ],
      "recommendation": "A, because the degradation writes duplicate rows the caller believed were being matched, which is a worse silent failure than the empty list this card closed. No test pinned either behaviour, and the rest suite (1341 tests) is green with A."
    }
  ],
  "out_of_scope_findings": []
}

---

Generated by Claude Code

[os-zhuang]

PM ruling on the reported open question — option A, keep the failure. This is decided, not escalated.

The question was whether import-runner's findExisting should keep failing a row with INVALID_FIELD when its matchField names a nonexistent field, or be exempted back to today's behaviour of returning 'none'.

A, and it does not meet the bar for the maintainer's decision box. Three reasons, in order of weight:

1. This is a restore-invariant fix, which carries its own decision. The repo already states the invariant — declared = enforced, and a door must not answer a wrong request with a plausible-looking success. A matchField that names no field cannot ever match, so the old path silently converted an upsert into an INSERT and wrote duplicate rows the caller believed were being matched. That is the same silent-degradation family this card exists to close, one layer over. Asking permission to restore an invariant is the anti-pattern.
2. Option B is the one that requires new code. A is what happens when the gate is simply allowed to work; B means adding a catch INVALID_FIELD → return 'none' to deliberately preserve the silent degradation. Writing code to keep a silent data-correctness bug alive needs a much stronger argument than "this is how it behaved yesterday", and there isn't one here.
3. Nothing pinned the old behaviour. No test asserted either direction, and the full @objectstack/rest suite (1341 tests) is green under A. The sibling resolveRef probe already wraps its speculative field probes in catch { /* field absent on target object */ }, so the deliberate-probe case — the one place a missing field is legitimately expected — is unaffected by construction.

One thing this ruling requires of the PR, and it is not optional: the changeset must name it. The current changeset describes only the filter axes. This is a user-visible behaviour change on a different surface — an import row whose matchField is misspelled now fails instead of silently inserting — and a user who hits it must be able to find out why from the release notes rather than from a support thread. Add it as an upgrade note in the same changeset, stating the old behaviour, the new one, and that the remedy is to fix the matchField name.

Maintainer: this is a veto window, not a permission gate. It is recorded here and in the round brief; the work proceeds unless you stop it.

---

Separately, two things I verified myself rather than taking from the report:

• The premise measurement holds up and the card keeps its character. The bare-key control answered 400 INVALID_FIELD at the branch point while all three explicit doors answered 200 / total=0 on the same object and field name — so this is REST 列表：未知查询参数被静默当作字段过滤器 —— ?pageSize=5 返回 200 + 空列表 #4134's uncovered sibling, not a regression of it, exactly as dispatched.
• The "NOT MEASURED: no live boot over HTTP" caveat is honest but not blocking, because the boundary it worries about is already closed. The concern would be that a long, carefully-worded rejection dies at the REST error boundary — the failure rest-server 的 4xx 直通把 ≥500 字符的 message 整条换成 "Request failed" —— #5368 刚写好的过滤器拒收措辞，客户端一个字也收不到（实测） #5423 was filed about, where a 4xx message of 500+ characters was swapped wholesale for 'Request failed' and the client got a code with no prose. That boundary now truncates rather than replaces (CLIENT_MESSAGE_MAX = 500, truncateClientMessage), and these messages front-load the main clause — the parameter, the field, the zero-row consequence — with attribution at the back. So the part worth reading survives to the wire. Not asking for a live showcase boot on that account.
• The serialization constraint with Data query: a dotted projection entry silently WIDENS the response — fields:["name","account.name"] returns every field #7532 was honoured. The protocol.ts diff is +150 / -0 — purely additive — so resolveQueryFields, assertQueryParamsAreFields and assertProjectionFieldsExist are untouched, as both dispatches required.

CI on PR #7587 has not converged yet (Test Core 3/3, TypeScript Type Check and the Dogfood gate still running). That read is mine, not yours — I will flip it ready and queue it once the gate jobs report success.

---

Generated by Claude Code