Skip to content

Commit ba4392e

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-6014-summary-comment-truth
2 parents 4c197c5 + 6513c17 commit ba4392e

12 files changed

Lines changed: 1545 additions & 25 deletions

File tree

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
"@objectstack/spec": major
3+
---
4+
5+
refactor(spec)!: `IDataDriver` 的 query 参数改为 `DriverQuery``Omit<QueryAST, 'object'>`),对象名只写一遍 (#5181)
6+
7+
`IDataDriver.find/findOne/count/updateMany/deleteMany/explain` 的第一个实参已经是对象名,而它们要求的 `QueryAST` 又把 `object` 列为必填 —— 同一个事实被要求写两遍,并因此有了两处互相矛盾的余地。上层为这份歧义已经付过账:objectql 引擎刻意把键序写成 `{ ...query, object }`,好让一个夹带的 `query.object` 覆盖不掉已解析的名字;wire 层则用一条具名 400(`QUERY_OBJECT_MISMATCH`)拒绝不一致。
8+
9+
驱动这一侧付的账是**成片的 cast**:一个手上只有 `where` 的直接调用方叫不出这个类型的名字,于是 `as any`,连带把 `where`/`orderBy`/`fields` 的类型检查一起关掉(cloud#1053 实测 20 处;cloud#1030 的 `$like` 就是从这个口子活到运行时的)。
10+
11+
**FROM → TO**
12+
13+
```ts
14+
// FROM —— 对象名写两遍
15+
await driver.find('account', { object: 'account', where: { status: 'open' } });
16+
// TO —— 第一个实参就是对象名
17+
await driver.find('account', { where: { status: 'open' } });
18+
```
19+
20+
一行修复:**删掉驱动调用字面量里的 `object:`**。编译器会把每一处指出来(TS2353 `'object' does not exist in type 'DriverQuery'`)。
21+
22+
**两个方向的兼容性,都不强迫任何一侧动**
23+
24+
- **调用方**:手上是一个 `QueryAST` ****的,原样传即可 —— 它具备 `DriverQuery` 要求的全部属性,多出来的那个在非新鲜字面量上 TypeScript 一律接受。新被拒绝的**恰好只是冗余本身**:写在调用点上、拼出 `object` 的内联字面量。本仓的迁移面因此实测只有 1 个文件 6 处(`@objectstack/metadata` 的 history-cleanup),已在同一 PR 里删除;引擎的 `driver.find(object, ast, …)` 一个字都不用改。
25+
- **驱动实现**:仍旧声明 `query: QueryAST` 的实现继续编译 —— 方法参数按双变比较。它们不再可以做的是**`query.object`**:调用方现在有权省略,声明会对一个运行时为 `undefined` 的值说谎。本仓五个驱动(memory / mongodb / sql / sqlite-wasm / turso)实测没有一个读它,因此本次不动驱动代码;把驱动签名一并迁到 `DriverQuery` 是后续的机械收尾。
26+
27+
`QueryAST` 的 zod 形状(`data/query.zod.ts``BaseQuerySchema`**没有动**`object` 在引擎与 hook 那一层是被读的,改的只是驱动契约的参数类型。`expand` 条目里的 `object` 同样保留 —— 那里它命名的是**关联对象**,没有任何实参携带这个事实,不是冗余。
28+
29+
标 major 是因为这是**源码级破坏性**变更(调用点字面量),运行时行为零变化。注意 `check:api-surface` 只看得见新增的 `DriverQuery` 导出、看不见参数类型的收窄(它记录导出存在与否,不记录签名),所以这条迁移说明是该变更唯一的下游载体。
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
---
2+
"@objectstack/service-datasource": patch
3+
---
4+
5+
fix(service-datasource): give this package's vitest run a 60s `testTimeout` — close the #4856 coverage hole that let the merge queue evict unrelated PRs (#6044)
6+
7+
`packages/services/service-datasource` had no `vitest.config.ts` at all, so
8+
every case ran under vitest's **5000ms** default. #4856 fixed this class of
9+
flake by setting per-package timeouts in each package's own `vitest.config.ts`
10+
— a structure that cannot reach a package with no config file to carry it.
11+
12+
The cases that build a REAL driver pay a one-time `@objectstack/driver-sql`
13+
(knex) import inside the first case that reaches it. In
14+
`datasource-pool-support.test.ts` the pool rejections throw before that import,
15+
so "sqlite WITHOUT a pool still builds exactly as before" is the first case
16+
through it: measured idle it runs ~1.1s while its neighbours run 0-2ms (the
17+
postgres/mysql cases ride the module cache at 31/82ms). ~4.6x headroom against
18+
5000ms holds on a PR branch and not on a merge-queue runner building several
19+
PRs' batches at once — the observed signature: intermittent reds only in queue
20+
full builds, evicting PRs that never touched this package (#5999 twice, #5973
21+
once, 2026-08-06).
22+
23+
`testTimeout: 60_000` reuses #4856's value rather than inventing a new number,
24+
set at the config layer so future cases are covered on arrival. Isolation was
25+
reviewed rather than assumed (the #6044 triage forbade a timeout-only closure):
26+
the flaky case builds `:memory:`, unprobed on the production path, never opens
27+
a connection or loads the native addon, and every factory-door case destroys
28+
its knex handle; the boot and wizard doors run on per-case fakes. No temp
29+
files, no ports, no shared mutable state across cases — the red was load
30+
variance on a real one-time import, not a leak.
31+
32+
No runtime, schema or public API change — test configuration only.

.github/workflows/validate-deps.yml

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,17 @@ jobs:
7272
# another (the 15.1.0 quickstart shipped exactly that: plugin-auth
7373
# declared better-auth ^1.6.23 while CI ran the 1.7.0-rc.1 override,
7474
# and every fresh project 500'd on auth).
75+
#
76+
# The same run also prints two informational censuses (#6046): overrides
77+
# nothing in the dependency tree consumes, and selectors whose upper
78+
# bound excludes their own target (#4961 / #5032). Both are REPORTS and
79+
# never fail the job — an unconsumed override is a legitimate posture
80+
# (#5835 ruling A). `--self-test` proves the check in both directions,
81+
# so run the `check:override-consistency` chain rather than the bare
82+
# script: a self-test nothing invokes is a phantom check.
7583
- name: Verify overrides are reflected in published manifests
76-
run: node scripts/check-override-consistency.mjs
77-
84+
run: pnpm check:override-consistency
85+
7886
# Fail the workflow if known vulnerabilities are found — enforces
7987
# security compliance before merging.
8088
#

package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
"check:docs-audit-scope": "node scripts/docs-audit/affected-docs.mjs --self-test && node scripts/docs-audit/check-audit-scope.mjs --self-test && node scripts/docs-audit/check-audit-scope.mjs",
3838
"check:role-word": "node scripts/check-role-word.mjs",
3939
"check:skill-frame-sync": "node scripts/check-skill-frame-sync.mjs --self-test && node scripts/check-skill-frame-sync.mjs",
40+
"check:skill-frame-freshness": "node scripts/check-skill-frame-freshness.mjs --self-test && node scripts/check-skill-frame-freshness.mjs",
4041
"check:adr-anchors": "node scripts/check-adr-anchors.mjs",
4142
"check:org-identifier": "node scripts/check-org-identifier.mjs",
4243
"check:authz-resolver": "node scripts/check-single-authz-resolver.mjs --self-test && node scripts/check-single-authz-resolver.mjs",
@@ -55,6 +56,7 @@
5556
"check:objectui-pin-fresh": "node scripts/check-objectui-pin-fresh.mjs --self-test && node scripts/check-objectui-pin-fresh.mjs",
5657
"check:prerelease-pins": "node scripts/check-prerelease-pin-watch.mjs --self-test && node scripts/check-prerelease-pin-watch.mjs",
5758
"check:empty-changeset": "node scripts/check-empty-changeset.mjs --self-test && node scripts/check-empty-changeset.mjs",
59+
"check:override-consistency": "node scripts/check-override-consistency.mjs --self-test && node scripts/check-override-consistency.mjs",
5860
"check:release-notes": "node scripts/check-release-notes.mjs",
5961
"check:release-body": "node scripts/release-github-releases.mjs --self-test",
6062
"check:node-version": "node scripts/check-node-version.mjs",

packages/metadata/src/utils/history-cleanup.ts

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,6 @@ export class HistoryCleanupManager {
126126
if (organizationId) baseWhere.organization_id = organizationId;
127127

128128
const metaItems = await driver.find(historyTableName, {
129-
object: historyTableName,
130129
where: baseWhere,
131130
fields: ['type', 'name'],
132131
});
@@ -148,7 +147,6 @@ export class HistoryCleanupManager {
148147
try {
149148
// Fetch only the IDs of records beyond the retention limit (oldest first)
150149
const historyRecords = await driver.find(historyTableName, {
151-
object: historyTableName,
152150
where: filter,
153151
orderBy: [{ field: 'version', order: 'desc' as const }],
154152
fields: ['id'],
@@ -192,7 +190,7 @@ export class HistoryCleanupManager {
192190
}
193191

194192
// Fallback: fetch IDs then delete
195-
const records = await driver.find(table, { object: table, where: filter, fields: ['id'] });
193+
const records = await driver.find(table, { where: filter, fields: ['id'] });
196194
const ids = records.map((r: Record<string, unknown>) => r.id as string).filter(Boolean);
197195
return this.bulkDeleteByIds(driver, table, ids);
198196
}
@@ -270,15 +268,13 @@ export class HistoryCleanupManager {
270268
}
271269

272270
recordsByAge = await driver.count(historyTableName, {
273-
object: historyTableName,
274271
where: filter,
275272
});
276273
}
277274

278275
// Count records that would be deleted by version limit
279276
if (this.policy.maxVersions) {
280277
const metaItems = await driver.find(historyTableName, {
281-
object: historyTableName,
282278
where: baseWhere,
283279
fields: ['type', 'name'],
284280
});
@@ -297,7 +293,6 @@ export class HistoryCleanupManager {
297293
const filter: Record<string, unknown> = { type, name, ...baseWhere };
298294

299295
const count = await driver.count(historyTableName, {
300-
object: historyTableName,
301296
where: filter,
302297
});
303298

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { defineConfig } from 'vitest/config';
4+
5+
export default defineConfig({
6+
test: {
7+
environment: 'node',
8+
// This package had no vitest config at all, so every case ran under
9+
// vitest's 5000ms default — the structural hole #4856 could not cover
10+
// (it set per-package timeouts in each package's own vitest.config.ts,
11+
// and this package had none). The cases that build a REAL driver pay a
12+
// one-time `@objectstack/driver-sql` (knex) import inside the first case
13+
// that reaches it: measured idle that case runs ~1.1s
14+
// (datasource-pool-support "sqlite WITHOUT a pool"), leaving ~4.6x
15+
// headroom that a loaded merge-queue runner eats — the #6044 signature
16+
// (green on every PR branch, intermittently red only in queue full
17+
// builds). 60s reuses #4856's value rather than inventing a new number,
18+
// set at the config layer so future cases are covered on arrival.
19+
testTimeout: 60_000,
20+
},
21+
});

packages/spec/api-surface/contracts.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@
6666
"DelegableAdminScope (interface)",
6767
"DelegableScope (interface)",
6868
"DeployExecutionResult (interface)",
69+
"DriverQuery (type)",
6970
"EMBEDDER_SERVICE (const)",
7071
"EmailAddress (type)",
7172
"EmailAttachment (interface)",

packages/spec/src/contracts/data-driver.test.ts

Lines changed: 121 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { describe, it, expect } from 'vitest';
2-
import type { IDataDriver } from './data-driver';
2+
import type { DriverQuery, IDataDriver } from './data-driver';
3+
import type { QueryAST } from '../data/query.zod';
4+
import type { DriverOptions } from '../data/driver.zod';
35

46
describe('IDataDriver', () => {
57
it('should allow creating a conforming mock implementation', () => {
@@ -143,4 +145,122 @@ describe('IDataDriver', () => {
143145
expect('findStream' in legacyShaped).toBe(true);
144146
});
145147
});
148+
149+
// ===========================================================================
150+
// DriverQuery — the AST no longer repeats the object name (#5181)
151+
// ===========================================================================
152+
//
153+
// Every pin below is resolved by tsc, not by vitest: reverting the change
154+
// (`query: DriverQuery` back to `query: QueryAST`) makes the `@ts-expect-error`
155+
// directives unused, and an unused directive is itself an error, so
156+
// `pnpm --filter @objectstack/spec typecheck` goes red. This file carries no
157+
// entry in `test-typecheck-debt.json`, which is what makes "zero errors" the
158+
// measurable baseline these pins move away from. The `expect()` calls only
159+
// give the assertions a home vitest will run.
160+
161+
describe('DriverQuery', () => {
162+
it('does not carry `object` at all — argument one is the only spelling', () => {
163+
type ObjectDropped = 'object' extends keyof DriverQuery ? never : 'dropped';
164+
const dropped: ObjectDropped = 'dropped';
165+
// Everything else survives: this is a subtraction of one key, not a new dialect.
166+
type WhereKept = 'where' extends keyof DriverQuery ? 'kept' : never;
167+
const kept: WhereKept = 'kept';
168+
expect([dropped, kept]).toEqual(['dropped', 'kept']);
169+
});
170+
171+
it('is what all six query-taking methods actually declare', () => {
172+
// This pin reads the parameter off the CONTRACT rather than off the alias,
173+
// and that is the point: a revert that puts `QueryAST` back on one
174+
// signature while leaving `DriverQuery` defined would sail past every
175+
// alias-scoped assertion in this block. Here that slot resolves to `never`
176+
// and the line goes red — per method, so the message names which one.
177+
type DropsObject<T> = 'object' extends keyof T ? never : 'dropped';
178+
const perMethod: [
179+
DropsObject<Parameters<IDataDriver['find']>[1]>,
180+
DropsObject<Parameters<IDataDriver['findOne']>[1]>,
181+
DropsObject<NonNullable<Parameters<IDataDriver['count']>[1]>>,
182+
DropsObject<Parameters<NonNullable<IDataDriver['updateMany']>>[1]>,
183+
DropsObject<Parameters<NonNullable<IDataDriver['deleteMany']>>[1]>,
184+
DropsObject<Parameters<NonNullable<IDataDriver['explain']>>[1]>,
185+
] = ['dropped', 'dropped', 'dropped', 'dropped', 'dropped', 'dropped'];
186+
expect(perMethod).toHaveLength(6);
187+
});
188+
189+
it('lets a caller pass only the query, which is what forced the casts', () => {
190+
// Before #5181 this literal did not compile (`object` was required), so a
191+
// caller holding just a `where` reached for `as any` — and lost the type
192+
// checking on everything else in the same stroke (cloud#1053, 20 sites).
193+
const q: DriverQuery = { where: { status: 'open' }, limit: 10 };
194+
expect(q.limit).toBe(10);
195+
});
196+
197+
it('rejects the redundant object key in a call-site literal', () => {
198+
// The excess-property check is the whole enforcement: writing the object
199+
// name twice is now a compile error rather than a convention nobody could
200+
// enforce. It is also what stops the two spellings from disagreeing —
201+
// the hazard the engine spends a key order on (`{ ...query, object }`)
202+
// and the wire layer spends a 400 on (`QUERY_OBJECT_MISMATCH`).
203+
// @ts-expect-error - 'object' does not exist in type 'DriverQuery'
204+
const redundant: DriverQuery = { object: 'account', where: { status: 'open' } };
205+
expect(redundant).toBeTruthy();
206+
});
207+
208+
it('still accepts a whole QueryAST value, so existing callers do not move', () => {
209+
// A `QueryAST` variable has every property `DriverQuery` requires and one
210+
// more; TypeScript admits the extra on any value that is not a fresh
211+
// literal. This is why the engine's `driver.find(object, ast, …)` needed
212+
// no edit — only literals written at the call site are re-judged.
213+
const ast: QueryAST = { object: 'account', where: { status: 'open' } };
214+
const asDriverQuery: DriverQuery = ast;
215+
expect(asDriverQuery.where).toEqual({ status: 'open' });
216+
});
217+
218+
it('keeps `object` inside an expand entry, where it is not redundant', () => {
219+
// The nested value names the RELATED object — a fact no argument carries.
220+
const q: DriverQuery = {
221+
fields: ['title'],
222+
expand: { owner: { object: 'user', fields: ['name'] } },
223+
};
224+
expect(q.expand?.owner?.object).toBe('user');
225+
226+
// @ts-expect-error - a nested expand entry still requires its own `object`
227+
const missing: DriverQuery = { expand: { owner: { fields: ['name'] } } };
228+
expect(missing).toBeTruthy();
229+
});
230+
231+
it('keeps an implementation that still declares the full QueryAST', () => {
232+
// Method parameters are compared bivariantly, so a driver written against
233+
// the old signature needs no edit to keep satisfying the contract. What it
234+
// may no longer do is READ `query.object` — callers are free to omit it —
235+
// and no driver in this repository does.
236+
const legacyImplementation: Pick<IDataDriver, 'find' | 'count'> = {
237+
async find(_object: string, _query: QueryAST, _options?: DriverOptions) {
238+
return [];
239+
},
240+
async count(_object: string, _query?: QueryAST, _options?: DriverOptions) {
241+
return 0;
242+
},
243+
};
244+
expect(legacyImplementation.count).toBeDefined();
245+
});
246+
247+
it('recovers the checks a blanket cast switched off — but not all of them', () => {
248+
// What the cast hid and this change gives back: the typed slots.
249+
// `orderBy` is `SortNode[]`, closed since #4721, so the `direction`
250+
// spelling that silently sorted the wrong way is a compile error again.
251+
// @ts-expect-error - spell the direction `order`, never `direction`
252+
const wrongSortKey: DriverQuery = { orderBy: [{ field: 'created_at', direction: 'desc' }] };
253+
expect(wrongSortKey).toBeTruthy();
254+
255+
// What it does NOT give back, stated here so nobody reads more into the
256+
// fix than it delivers: `where` is `FilterCondition`, whose index
257+
// signature is `[key: string]: any` because ANY field name is a legal key.
258+
// An operator the dialect does not have is therefore still not a type
259+
// error — `$like` (cloud#1030) reaches the runtime filter compiler and is
260+
// rejected there, not here. Removing the cast does not close that door;
261+
// only a closed operator vocabulary would, which is a separate change.
262+
const unknownOperator: DriverQuery = { where: { name: { $like: 'acme%' } } };
263+
expect(unknownOperator.where).toBeTruthy();
264+
});
265+
});
146266
});

0 commit comments

Comments
 (0)