Skip to content

Commit 7bb8084

Browse files
fix(client): data.find refuses a select that names no field before the request (#26946)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XBJqmquET4esJZNQWTR5J1e)_ Fixes #26910 Clause-②: no Metadata-protocol: none The `Clause-②` measure: the calls refused are exactly the calls the list door already refuses with `400 VALIDATION_ERROR` (the parity test below sends the same request to the real door), and the caller gets the same `code`, `httpStatus` and message, only without the round trip. No call that was served is refused, nothing is widened, and no type changes (`select` / `fields` stay `string[]`; no non-empty tuple narrowing was added). ## 维护者速读 - 改了什么:SDK 的 `data.find` 遇到 `select: []`(或 `fields: []`、成员全为空白)时,在发请求之前就拒绝,报错与服务端列表门完全相同(`VALIDATION_ERROR` / 400 / 同一句话)。 - 为什么改:之前空数组为真值,SDK 会发出 `?select=`,白跑一趟网络再拿到 400;改成调用点直接报错。参数绝不丢弃,丢弃会变成读全部列。 - 风险与回滚:只影响本来就会被服务端 400 拒绝的调用;回滚即还原 `packages/client/src/index.ts` 中的一个 helper 及两处调用。 - 先看:`packages/client/src/index.ts` 的 `projectionSelectParam`;`packages/client/src/client.hono.test.ts` 新增的对等测试。 - 两分钟验证:`pnpm --filter @objectstack/client exec vitest run src/client.test.ts src/client.hono.test.ts`,预期全部通过,其中含 4 条空 projection 拒绝用例与 1 条真实门对等用例。 - 需要你做的:无,席位落地。 ## What changes - **One shared helper, both copies.** `projectionSelectParam` (`packages/client/src/index.ts`) builds the `select` value both `ObjectStackClient.data.find` and `ScopedEnvironmentClient.data.find` put on the wire. It applies the door's own rule to the exact value the client would have sent (split on commas, trim, at least one name) and, when that names no field, throws before any request with `code: 'VALIDATION_ERROR'`, `httpStatus: 400` and the door's sentence (`Query parameter 'select' names no field; got "". The query was not run: ...`). - **Never dropped.** An empty projection is refused, not omitted: an absent `select` reads every column, the widening the door refuses. - **Where the sentence comes from.** The door's copy lives in `projectionFieldsValue`, a module-private function in `packages/metadata-protocol/src/protocol.ts`. No shared copy exists in a package the client may import (`@objectstack/client` depends on `@objectstack/core` and `@objectstack/spec` only; spec is outside this card's file surface, and the spec's `HttpFindQueryParamsSchema.select` message is a different sentence). So the client restates it, and `client.hono.test.ts` holds the restated copy equal to the real door's answer. - **Sweep.** Every `select` emitter in `packages/client/src`: exactly the two `data.find` copies (`queryParams.set('select', ...)` at the two sites, both reached by the canonical `fields` mapping too). No other method builds `select` or another projection parameter from a truthy test: `data.query` sends a QueryAST body verbatim, and `QueryBuilder.select()` writes `fields` into that body (the POST door applies the same refusal server-side). - Changeset: `patch` on `@objectstack/client`. ## Tests - **Red first, on `main` `8074feb6`.** A probe drove both copies with a stubbed `fetch`: `{ select: [] }` and `{ fields: [] }` requested `.../data/task?select=` (scoped: `.../environments/env-1/data/task?select=`), `{ select: ['', ' '] }` requested `?select=%2C+`. The new `client.test.ts` cases went red there: `4 failed | 1 passed` with `promise resolved ... instead of rejecting`. - **Green, on `8efd86c3`.** `client.test.ts`: four rows (`select: []`, `fields: []`, `select: ['', ' ']`, `fields: [' ']`), each on both copies, assert the rejection's `code`, `httpStatus` and message and that `fetch` was never called. Controls: `select: ['a','b']` and `fields: ['a','b']` send `select=a%2Cb`; `select: ['a', '']` still sends `select=a%2C` (the door serves it); the existing `no options at all` row still sends no parameter. - **Parity with the real door.** `client.hono.test.ts` sends `?select=` and `?select=%2C+` raw through the client's transport against the real REST stack, then asserts `data.find` refuses `[]` and `['', ' ']` with the identical `message`, `code` and `httpStatus`. - **Ablation (one-shot, not kept).** `scripts/ablation-replace.mjs` drifted the restated sentence's last clause in `index.ts`; the parity test went red (`1 failed`); restore proved by blob `e553346c4fe0` equal to `HEAD` and an empty `git diff HEAD`. - `@objectstack/client`: `pnpm test` 52 files, 678 tests passed; `typecheck` exit 0 (including `check:test-typecheck`); `eslint packages/client --no-inline-config` exit 0. ## Acceptance notes - `groupBy` is emitted on the same truthy test (`groupBy: []` sends `?groupBy=`). It is not a projection and the card names only projections, so it is untouched here. - A JS caller passing a string instead of the declared `string[]` hits `select.join is not a function`, as before; nothing about that path changes. - No open pull request titled for the client was found (`is:open client in:title`, 0 hits), so no overlap on `packages/client/src/index.ts` was seen. ## Verification <details><summary>Derived gate families on 8efd86c (59 derived, 58 run, 1 NOT MEASURED)</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 59 families on this diff; `--ran` reconciled 59 accounted, 58 run with exit 0, 1 NOT MEASURED. - NOT MEASURED: `pnpm check:dual-build-cjs-loads` exited 3 (PREREQUISITE NOT MET: it needs every package's `dist/`, i.e. a full workspace build). CI's `Lint & Repo Gates` runs it on this PR. - `node scripts/check-plugin-teardown-shape.mjs --self-test` first exited 3 on the shallow clone; after fetching its pinned fixture commit it exited 0 (48 cases). - `pnpm --filter @objectstack/spec run check:skill-examples` first exited 3 (client-react not built); after building it, exit 0 (267 prose examples type-check, 23 blocks on the client SDK surface). - Every other family: exit 0. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5mTFyYsPjawe7Bv7x5)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FEkQ5mTFyYsPjawe7Bv7x5 Co-authored-by: Claude <noreply@anthropic.com>
1 parent eac28c3 commit 7bb8084

4 files changed

Lines changed: 134 additions & 3 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@objectstack/client': patch
3+
---
4+
5+
fix(client): `data.find` refuses a `select` that names no field before the request, with the list door's own answer, instead of sending `?select=`
6+
7+
Clause-②: no — the calls refused are exactly the ones the list door already refuses (`400 VALIDATION_ERROR`); the caller gets the same `code`, `httpStatus` and message, only without the round trip. Nothing is widened and no call that was served is refused.
8+
Metadata-protocol: none
9+
10+
An empty array is truthy, so `client.data.find(object, { select: [] })` (or `{ fields: [] }`) put `?select=` on the wire and came back as a `400 VALIDATION_ERROR` from a door the caller cannot see. Both `ObjectStackClient.data.find` and `environment(id).data.find` now refuse that call, and a `select` whose every member is blank, before any request:
11+
12+
- The error carries `code: 'VALIDATION_ERROR'`, `httpStatus: 400` and the door's sentence (`Query parameter 'select' names no field; …`), so an existing `err.code` branch fires the same way.
13+
- The parameter is never dropped: an absent projection reads every column, which is the widening the door refuses.
14+
- A `select` naming at least one field is sent exactly as before, blank members included (the door keeps the named ones).

‎packages/client/src/client.hono.test.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,33 @@ describe('ObjectStackClient (with Hono Server)', () => {
309309
expect(resultsResponse.records[0].name).toBe('Hono User');
310310
});
311311

312+
// `data.find` refuses a projection naming no field before the request, with
313+
// a restated copy of the list door's sentence (the door's own is private to
314+
// the server). This holds the copy equal to the REAL door: the same request
315+
// sent raw through the client's transport — which is what `find` did before
316+
// it refused — must fail with the same message, code and status.
317+
it('refuses an empty select with the list door\'s own answer, before the request', async () => {
318+
const client = new ObjectStackClient({ baseUrl });
319+
await client.connect();
320+
321+
for (const select of [[], ['', ' ']]) {
322+
const wire = new URLSearchParams({ select: select.join(',') }).toString();
323+
const door = await client._fetch(`${baseUrl}/api/v1/data/customer?${wire}`).then(
324+
() => { throw new Error(`the door served ?${wire}`); },
325+
(err: any) => err,
326+
);
327+
expect(door.httpStatus).toBe(400);
328+
expect(door.code).toBe('VALIDATION_ERROR');
329+
330+
const refused = await client.data.find('customer', { select }).then(
331+
() => { throw new Error('find resolved'); },
332+
(err: any) => err,
333+
);
334+
expect({ message: refused.message, code: refused.code, httpStatus: refused.httpStatus })
335+
.toEqual({ message: door.message, code: door.code, httpStatus: door.httpStatus });
336+
}
337+
});
338+
312339
// [#5638] The one method whose declared return shape this suite never
313340
// exercised — and the one that was wrong. `DeleteDataResult` claimed
314341
// `deleted: boolean`; the schema it names declares `success`. This is the

‎packages/client/src/client.test.ts‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2045,6 +2045,62 @@ describe('data.find() — canonical/legacy transport parameters (both copies)',
20452045
);
20462046
expect(b.fetchMock).not.toHaveBeenCalled();
20472047
});
2048+
2049+
/**
2050+
* A projection that names no field is refused before the request, on both
2051+
* copies — never dropped.
2052+
*
2053+
* An empty array is truthy, so `{ select: [] }` used to reach the wire as
2054+
* `?select=`, and the list door refused it with `400 VALIDATION_ERROR`
2055+
* after the round trip. Dropping the parameter instead would be worse: an
2056+
* absent projection reads every column, the widening the door refuses.
2057+
* The client now answers what the door would have answered — the same
2058+
* `code`, `httpStatus` and sentence (parity with the real door is pinned
2059+
* in `client.hono.test.ts`) — and sends nothing.
2060+
*/
2061+
const EMPTY_PROJECTIONS: Record<string, { options: QueryOptions | QueryOptionsV2; wire: string }> = {
2062+
'legacy { select: [] }': { options: { select: [] }, wire: '' },
2063+
'canonical { fields: [] }': { options: { fields: [] }, wire: '' },
2064+
'legacy, every member blank': { options: { select: ['', ' '] }, wire: ', ' },
2065+
'canonical, every member blank': { options: { fields: [' '] }, wire: ' ' },
2066+
};
2067+
2068+
for (const [name, row] of Object.entries(EMPTY_PROJECTIONS)) {
2069+
it(`refuses an empty projection before any request on both copies: ${name}`, async () => {
2070+
const expected = {
2071+
code: 'VALIDATION_ERROR',
2072+
httpStatus: 400,
2073+
message: `Query parameter 'select' names no field; got ${JSON.stringify(row.wire)}. The query was not run: `
2074+
+ 'an empty projection was read as no projection, which answers every column. Name at least one '
2075+
+ 'field, or leave the parameter out to read every column.',
2076+
};
2077+
2078+
const a = createMockClient({ success: true, data: { object: 'task', records: [] } });
2079+
await expect(a.client.data.find('task', row.options)).rejects.toMatchObject(expected);
2080+
expect(a.fetchMock).not.toHaveBeenCalled();
2081+
2082+
const b = createMockClient({ success: true, data: { object: 'task', records: [] } });
2083+
await expect(b.client.environment('env-1').data.find('task', row.options)).rejects.toMatchObject(expected);
2084+
expect(b.fetchMock).not.toHaveBeenCalled();
2085+
});
2086+
}
2087+
2088+
it('still sends a projection that names at least one field, blank members and all, on both copies', async () => {
2089+
const body = { success: true, data: { object: 'task', records: [], total: 0 } };
2090+
for (const options of [{ select: ['a', 'b'] }, { fields: ['a', 'b'] }] as const) {
2091+
const a = createMockClient(body);
2092+
await a.client.data.find('task', options);
2093+
expect(queryOf(a.fetchMock.mock.calls[0][0] as string)).toBe('select=a%2Cb');
2094+
const b = createMockClient(body);
2095+
await b.client.environment('env-1').data.find('task', options);
2096+
expect(queryOf(b.fetchMock.mock.calls[0][0] as string)).toBe('select=a%2Cb');
2097+
}
2098+
// The door splits, trims and keeps the named members, so this one is
2099+
// served — the client sends it exactly as before.
2100+
const { direct, scoped } = await driveBoth({ select: ['a', ''] });
2101+
expect(direct).toBe('select=a%2C');
2102+
expect(scoped).toBe('select=a%2C');
2103+
});
20482104
});
20492105

20502106
describe('QueryBuilder — offset() alias', () => {

‎packages/client/src/index.ts‎

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,39 @@ function canonicalExpandParam(expand: QueryOptionsV2['expand']): string | undefi
417417
return names.length > 0 ? names.join(',') : undefined;
418418
}
419419

420+
/**
421+
* The `select` value `data.find()` puts on the wire, or the list door's own
422+
* refusal, raised before any request.
423+
*
424+
* The door (`findData` in `@objectstack/metadata-protocol`) splits `select` on
425+
* commas, trims each member and refuses a value naming NO field with
426+
* `400 VALIDATION_ERROR`: an empty projection read as no projection answers
427+
* every column. An empty array is truthy, so `{ select: [] }` used to reach the
428+
* wire as `?select=` and come back as that 400 after a round trip. Dropping
429+
* the parameter instead is the one thing this must never do — absent means
430+
* "every column", the widening the door exists to refuse.
431+
*
432+
* The rule and the sentence are the door's, applied to the exact value this
433+
* client would have sent, so the caller gets the same `code`, `httpStatus`
434+
* and message either way. The sentence is restated here because the door's
435+
* copy is private to a server package this SDK does not depend on;
436+
* `client.hono.test.ts` holds the two equal against the real door.
437+
*
438+
* Shared by both `data.find` copies so they cannot diverge.
439+
*/
440+
function projectionSelectParam(select: readonly string[]): string {
441+
const value = select.join(',');
442+
if (value.split(',').some((name) => name.trim() !== '')) return value;
443+
const error = new Error(
444+
`Query parameter 'select' names no field; got ${JSON.stringify(value)}. The query was not run: `
445+
+ 'an empty projection was read as no projection, which answers every column. Name at least one '
446+
+ 'field, or leave the parameter out to read every column.',
447+
) as Error & { code: string; httpStatus: number };
448+
error.code = 'VALIDATION_ERROR';
449+
error.httpStatus = 400;
450+
throw error;
451+
}
452+
420453
export interface PaginatedResult<T = any> {
421454
/** Spec-compliant: array of matching records */
422455
records: T[];
@@ -7056,9 +7089,10 @@ export class ObjectStackClient {
70567089
}
70577090
}
70587091

7059-
// 3. Handle Select
7092+
// 3. Handle Select — a projection naming no field is refused here,
7093+
// never dropped (see projectionSelectParam).
70607094
if (normalizedOptions.select) {
7061-
queryParams.set('select', normalizedOptions.select.join(','));
7095+
queryParams.set('select', projectionSelectParam(normalizedOptions.select));
70627096
}
70637097

70647098
// 4. Handle Filters (Simple vs AST)
@@ -7944,7 +7978,7 @@ export class ScopedEnvironmentClient {
79447978
}
79457979
}
79467980
if (normalizedOptions.select) {
7947-
queryParams.set('select', normalizedOptions.select.join(','));
7981+
queryParams.set('select', projectionSelectParam(normalizedOptions.select));
79487982
}
79497983
const filterValue = normalizedOptions.filter ?? normalizedOptions.filters;
79507984
if (filterValue) {

0 commit comments

Comments
 (0)