Skip to content

Commit 4caf9e6

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-20387-aggregate-one-double
2 parents 36d7b2a + 8e02859 commit 4caf9e6

15 files changed

Lines changed: 748 additions & 120 deletions
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
---
2+
"@objectstack/driver-turso": minor
3+
---
4+
5+
`TursoDriver` in **remote** mode refuses a read over a missing table or a missing column with the same code the local mode answers, instead of answering "no rows" (#20424).
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (already-registered driver-sql-unresolvable-where-column-refused) That registered entry names `TursoDriver` among the `SqlDriver` subclasses whose reads now refuse an unresolvable column, and prescribes this change's remedy: name a column the table has, or run schema sync so a declared field exists as a column. The `aggregate` legs (a table that is really absent, a `groupBy` or aggregation column that is absent) are the same drifted-schema family with the same remedy, so no new migration entry is owed. -->
10+
11+
**BREAKING** — an accept-set narrowing on the remote face of `TursoDriver`'s read doors, shipped as `minor` under the launch-window convention (`check-changeset-no-major` refuses `major` until GA; breaking-ness is carried by this banner and the ADR-0087 disposition above, not by the level). Remote-face users meet this refusal for the first time here.
12+
13+
**FROM → TO.** A remote-face read (`aggregate`, `find`, `findOne`, `count`) that answered `[]` / `null` for a missing table or a missing column now refuses, as the local face does: `DATABASE_ERROR` / 500 for a table that is absent, `INVALID_FIELD` / 400 for a `groupBy` or aggregation column that is absent, `INVALID_FILTER` / 400 for a `where` column that is absent. **The fix:** run schema sync so the declared field has its column (or the object its table), or name a column the table has. `RemoteTransport.find` and `RemoteTransport.aggregate`, exported from the package root, now raise the backend's error where they answered `[]`.
14+
15+
**What was wrong.** Two catches in `RemoteTransport` read a backend "no such table" or "no such column" as an empty result. `aggregate` answered `[]` for both. `find` (and `findOne` through it) answered `[]` (`null`) for a missing column once its projection retry was spent, or when there was no projection to drop. So on a remote Turso database a schema drift or a missing table read as "there is no data", while the local mode of the same driver, over the same file, refused it. Measured with a local driver over the same libSQL file as the control, for a federated and for a managed object alike:
16+
17+
| read | local | remote before |
18+
|:--|:--|:--|
19+
| `aggregate` on a table that is really absent | `DATABASE_ERROR` / 500 | `[]` |
20+
| `aggregate` grouped by, or aggregating, a declared field whose column is absent | `INVALID_FIELD` / 400 | `[]` |
21+
| `aggregate` whose `where` names that field | `INVALID_FILTER` / 400 | `[]` |
22+
| `find` / `findOne` whose `where` names that field | `INVALID_FILTER` / 400 | `[]` / `null` |
23+
| `count` whose `where` names that field | `INVALID_FILTER` / 400 | `DATABASE_ERROR` / 500 |
24+
| `find` ordered by that field | the rows, unordered | `[]` |
25+
26+
**What changes, on the remote face only:**
27+
28+
- `aggregate`, `find`, `findOne` and `count` answer each row above the way the local face does. The backend's error is classified by the local face's own inherited seam, `SqlDriver.aggregateBackendFault`, and not by a second copy: an unresolvable column named by a `groupBy` or an aggregation is `INVALID_FIELD` / 400, one named by the `where` is `INVALID_FILTER` / 400, and anything else is `DATABASE_ERROR` / 500. The dialect text goes to the server log, never to the caller.
29+
- `find` keeps the local face's recovery ladder: a projection naming a column the table lacks is dropped first, then an ORDER BY on one, and the rows answer. A `where` is never dropped. Before, the ORDER BY rung was missing and the sort answered `[]`.
30+
- A refusal the transport raises while it compiles the statement (a filter or aggregate-vocabulary refusal, the timeout envelope) keeps its own code and status.
31+
- `RemoteTransport.find` and `RemoteTransport.aggregate`, used on their own, now raise the backend's error where they answered `[]`.
32+
33+
This is the refusal the registered migration entry `driver-sql-unresolvable-where-column-refused` already names for `driver-sql` "and its `TursoDriver` / `SqliteWasmDriver` subclasses": the remote face of `TursoDriver` now delivers it. **If a read now refuses for you:** the table or column it names is missing from the remote database. Run schema sync so the declared field has its column (or the object its table), or correct the name the query uses.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/rest": patch
3+
---
4+
5+
**`GET /api/v1/meta/:type/:name/audit` is now an authoring door: a caller without an authoring capability is refused, as `/diff`, `/history` and `GET /api/v1/meta/_drafts` refuse.** Before this release, any signed-in caller who could open an item could read its protection-audit trail. Every save appends a row to that trail, a draft save included, and the row carries `note: "draft"`, the actor and the time. So a member could learn that an item had unpublished authoring work, who saved it and when. For an item that had never been published, where the plain read answers `404`, the member could learn that it existed at all. This carries the maintainer's ruling on #20378 (letter B, comment 5865708652) to this door, as triage graded on #20441: draft and preview reads are admin-gated upstream (ADR-0106 D4), and the audit trail, like the version log, has no published-only answer to fall back to.
6+
7+
Clause-②: no
8+
9+
- **Who may read it:** a system context, or a caller holding `studio.access`, `setup.access` or `manage_metadata`. This is the predicate `/meta/_drafts`, `/diff`, `/history` and every draft switch already ask, not a second rule.
10+
- **Everyone else:** `403` with code `FORBIDDEN`, in the same nested `error` envelope `/meta/_drafts` answers. The refusal is decided on the caller before the protocol is resolved, before the query is parsed and before any event is read. So it is the same answer for an item that exists, one that does not, and one that exists only as a draft, and it carries no event, actor or item name. The message names the door, not drafts.
11+
- **Unchanged:** callers with an authoring capability read the trail exactly as before, including the per-caller refusal of an item the plain read refuses them and the organization scope of the read.
12+
13+
A client that read `/audit` (`client.meta.getAudit`) as a member now receives `403 FORBIDDEN`. To read it, call as a caller holding one of the three capabilities above.

‎content/docs/api/client-sdk.mdx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ const diff = await client.meta.diffItem('object', 'account', { from: 2, to: 5 })
233233
// Introspection & governance
234234
const bad = await client.meta.getDiagnostics({ severity: 'error' });
235235
const refs = await client.meta.getReferences('object', 'account');
236+
// Authoring-only too, like diffItem: without studio.access, setup.access or manage_metadata → 403 FORBIDDEN
236237
const trail = await client.meta.getAudit('object', 'account', { limit: 20 });
237238
const tree = await client.meta.getBookTree('handbook');
238239

‎content/docs/ui/apps.mdx‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -379,12 +379,13 @@ an app an author may not open is refused on these doors too. Reading a
379379
needs an authoring capability (`studio.access`, `setup.access` or
380380
`manage_metadata`: the check `GET /api/v1/meta/_drafts` makes). A caller
381381
without one is answered as if the parameter were absent: the published app,
382-
or `404` for an app that has never been published. `/diff`, and the change log
383-
`/history` beside it, need that capability outright: both read the version log,
384-
which records a draft save like any other, so they have no published-only
385-
answer to fall back to. A caller without one is refused with `403`, as
386-
`GET /api/v1/meta/_drafts` refuses, before anything is read — the same answer
387-
whether or not the app exists.
382+
or `404` for an app that has never been published. `/diff`, the change log
383+
`/history` beside it and the protection-audit trail `/audit` need that
384+
capability outright: the version log and the audit trail each record a draft
385+
save like any other, so none of them has a published-only answer to fall back
386+
to. A caller without one is refused with `403`, as `GET /api/v1/meta/_drafts`
387+
refuses, before anything is read — the same answer whether or not the app
388+
exists.
388389

389390
`visible` did **not** move server-side with them, and that asymmetry is
390391
deliberate: CEL is evaluated in the browser because server-side evaluation needs

‎packages/drivers/driver-turso/src/remote-transport-unknown-select.test.ts‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,23 @@ describe('RemoteTransport unknown-$select column', () => {
5151
expect(calls[1].sql).toMatch(/SELECT \* FROM "product"/);
5252
});
5353

54-
it('still returns empty when even SELECT * fails (e.g. unknown table)', async () => {
55-
const { t } = transportWithClient(async () => {
54+
// [#20424] This case REPLACES the pin 'still returns empty when even SELECT *
55+
// fails (e.g. unknown table)', which asserted `[]` for exactly this input.
56+
// `[]` was the defect: a column the WHERE still names once the projection is
57+
// gone is a predicate that never ran, and "no rows" is a false answer to it.
58+
// The transport now raises the backend's error from the last rung, and
59+
// `TursoDriver` classifies it (`INVALID_FILTER` / 400, the local face's
60+
// answer). The same input, the opposite assertion.
61+
it('raises the last rung\'s error when even SELECT * fails, instead of answering []', async () => {
62+
const { t, calls } = transportWithClient(async () => {
5663
throw new Error('SQLITE_ERROR: no such column: status');
5764
});
58-
const result = await t.find('ghost', { fields: ['id', 'status'], limit: 10 });
59-
expect(result).toEqual([]);
65+
await expect(t.find('ghost', { fields: ['id', 'status'], limit: 10 })).rejects.toThrow(
66+
/no such column: status/,
67+
);
68+
// The projection attempt, then the one rung this query has.
69+
expect(calls).toHaveLength(2);
70+
expect(calls[1].sql).toMatch(/SELECT \* FROM "ghost"/);
6071
});
6172

6273
it('propagates non-column errors instead of hiding them as empty', async () => {

‎packages/drivers/driver-turso/src/remote-transport.ts‎

Lines changed: 45 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1412,16 +1412,39 @@ export class RemoteTransport {
14121412
// real rows still come back; the unknown field is simply absent from
14131413
// each row (it never existed). Mirrors the SqlDriver backstop — the
14141414
// remote Turso path overrides find(), so it needs its own copy.
1415+
//
1416+
// [#20424] The copy now has the local ladder's two rungs and its
1417+
// terminal. The rungs, in `SqlDriver.findRows`' order: the projection
1418+
// first, then the ORDER BY (#3821: rows matter more than their order),
1419+
// each rebuilt with the caller's WHERE, which neither rung may drop.
1420+
// The terminal was `return []` here, on both the no-rung path and a
1421+
// failed rung, so an unresolvable column in the WHERE, or an ORDER BY
1422+
// on a column the table lacks, read as "there are no rows". The local
1423+
// face answers the first with `INVALID_FILTER` / 400 (#8790) and the
1424+
// second with its rows, unordered. Now the last error leaves this
1425+
// method as the backend raised it, and `TursoDriver` classifies it
1426+
// with the local face's own read-exit seam. The refusal and its code
1427+
// are the driver's, not this transport's.
1428+
const rungs: any[] = [];
14151429
if (query?.fields && Array.isArray(query.fields) && query.fields.length > 0) {
1430+
rungs.push({ ...query, fields: undefined });
1431+
}
1432+
if (Array.isArray(query?.orderBy) && query.orderBy.some((item: any) => item?.field)) {
1433+
rungs.push({ ...query, fields: undefined, orderBy: undefined });
1434+
}
1435+
let lastError: unknown = error;
1436+
for (const rung of rungs) {
14161437
try {
1417-
const fallback = this.buildSelectSQL(object, { ...query, fields: undefined }, table);
1438+
const fallback = this.buildSelectSQL(object, rung, table);
14181439
const result = await this.client!.execute({ sql: fallback.sql, args: fallback.args });
14191440
return this.mapRows(result);
1420-
} catch {
1421-
return [];
1441+
} catch (rungError) {
1442+
// The next, broader rung. The last one to fail names the column
1443+
// the WHERE still holds, once the projection and the sort are gone.
1444+
lastError = rungError;
14221445
}
14231446
}
1424-
return [];
1447+
throw lastError;
14251448
}
14261449
throw error;
14271450
}
@@ -1626,19 +1649,18 @@ export class RemoteTransport {
16261649
sql += ` GROUP BY ${groupBy.map((g) => `"${g.field}"`).join(', ')}`;
16271650
}
16281651

1629-
try {
1630-
const result = await this.client!.execute({ sql, args });
1631-
return this.foldEmptyAggregateAnswers(this.mapRows(result), foldedOutput);
1632-
} catch (error: any) {
1633-
if (
1634-
error.message &&
1635-
(error.message.includes('no such table') ||
1636-
error.message.includes('no such column'))
1637-
) {
1638-
return [];
1639-
}
1640-
throw error;
1641-
}
1652+
// [#20424] The backend's error leaves this method as it was raised. A
1653+
// catch here used to answer `no such table` and `no such column` with `[]`,
1654+
// so a table that is really absent, or a groupBy, aggregation or WHERE
1655+
// naming a declared field whose column is absent, read as "no data". The
1656+
// local face of the same driver refuses all of them. `TursoDriver`
1657+
// classifies the error with the local face's own aggregate seam
1658+
// (`SqlDriver.aggregateBackendFault`): `INVALID_FIELD` / 400 for a groupBy
1659+
// or aggregation column, `INVALID_FILTER` / 400 for a WHERE column, and
1660+
// `DATABASE_ERROR` / 500 for the rest. The catch arrived with this method's
1661+
// first version, with no comment, no pin and no case it was written for.
1662+
const result = await this.client!.execute({ sql, args });
1663+
return this.foldEmptyAggregateAnswers(this.mapRows(result), foldedOutput);
16421664
}
16431665

16441666
/**
@@ -2883,11 +2905,12 @@ export class RemoteTransport {
28832905
//
28842906
// The first two are the silent empty set: SQLite's backwards-compatible
28852907
// rule degrades a double-quoted name that resolves to no column into a
2886-
// STRING LITERAL, so the statement compiles, runs and matches nothing —
2887-
// and where a build disables that rule (`SQLITE_DQS=0`) `find()`'s own
2888-
// `no such column` backstop swallows the error into `[]` anyway. Two
2889-
// roads, one answer, and neither is distinguishable from "no rows
2890-
// matched".
2908+
// STRING LITERAL, so the statement compiles, runs and matches nothing,
2909+
// which is not distinguishable from "no rows matched". Where a build
2910+
// disables that rule (`SQLITE_DQS=0`), the statement fails with
2911+
// `no such column` instead, and `find()`'s backstop now refuses it, as
2912+
// the local face does (`INVALID_FILTER` / 400, see #20424). It used to
2913+
// swallow that error into `[]` too: a second road to the same answer.
28912914
//
28922915
// The third is the expensive direction, and it needs no dialect quirk at
28932916
// all: a `{}` disjunct absorbs its `$or` to TRUE, the compiled clauses are

0 commit comments

Comments
 (0)