Skip to content

Commit 050c5ba

Browse files
committed
fix(objectql,cli): the integrity audit stops when the engine does (#4747)
Every `os migrate` subcommand ended a SUCCESSFUL run with two `ERROR Find operation failed` lines and a #4551 report naming `sys_metadata` / `sys_view_definition` as `unreadableObjects`. That bucket exists to separate "I could not check" from "I checked and it was fine"; non-empty on every healthy run, it separated nothing. Two silent no-ops stacked up: - `ObjectQLPlugin` put its teardown in `stop()`, but the kernel's Plugin contract is `init`/`start`/`destroy` — `stop()` is never called by anyone, so the ADR-0057 sweep timers were never disarmed on any host. - `bootSchemaStack().shutdown()` called `(runtime as any).stop?.()` and `Runtime` has no `stop`. The optional call made "no teardown" look exactly like "teardown performed": the kernel stayed running with every timer armed while the command closed its driver. 60s after boot the sweep woke inside the still-alive process and read a pool its own host had already disconnected. - ObjectQLPlugin: `stop` -> `destroy`, the hook the kernel actually calls. - bootSchemaStack: tear down via `kernel.shutdown()` — the same path `os serve` takes on SIGTERM, so one-shot and server exit alike. - LifecycleService.stop(): raise an abort bit the in-flight sweep holds, not just clear timers; refuse to start a sweep once stopped. - The audit takes that bit as `signal` and stops issuing reads. A read that fails *because* the run was called off is dropped rather than filed — it is not evidence about the datasource. The new `DanglingReferenceReport.aborted` keeps the incompleteness loud without spending the finding bucket on it. A genuine datasource fault still lands in `unreadableObjects`, and the audit still runs on CLI hosts: this is a liveness boundary, not the "one-shot commands skip the audit" switch the issue rejected. Fixes #4747 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny
1 parent 25784cf commit 050c5ba

9 files changed

Lines changed: 620 additions & 15 deletions

File tree

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
---
2+
'@objectstack/objectql': patch
3+
'@objectstack/cli': patch
4+
---
5+
6+
修复:每个 `os migrate` 子命令关停后,#4551 悬空引用巡检都会把 `sys_metadata` / `sys_view_definition` 报成 `unreadableObjects`(#4747)
7+
8+
一条**成功**的命令过去会在返回 JSON 之后打出两行 `ERROR Find operation failed` 和一份
9+
`unreadableObjects` 非空的巡检报告 —— 对抓 ERROR 的 CI 流水线是直接误报源,更要命的是它把
10+
`unreadableObjects` 变成了恒为真的告警:那个桶存在的意义正是区分「我没能检查」和「我检查了,
11+
没问题」,一个每次健康运行都非空的桶不再携带任何信息。
12+
13+
两处静默空转叠出了这个结果:
14+
15+
- `ObjectQLPlugin` 的关停逻辑写在 `stop()` 里,而内核的插件契约是 `init`/`start`/`destroy` ——
16+
`stop()` 从来没有被任何人调用过,ADR-0057 巡检定时器因此在任何宿主上都不会被解除。改为
17+
`destroy()`(与 `DefaultDatasourcePlugin` 一致)。
18+
- `bootSchemaStack().shutdown()` 调的是 `(runtime as any).stop?.()`,而 `Runtime` 根本没有
19+
`stop` —— 可选调用把「没有关停」伪装成了「关停过了」。改为走内核自己的 `kernel.shutdown()`,
20+
`os serve` 收到 SIGTERM 时同一条路径。
21+
22+
同时 `LifecycleService.stop()` 不再只是清定时器:它还会把「引擎正在拆」这一位交给正在飞行中的
23+
sweep,巡检据此在读之前停手。因关停而失败的读**不再进入** `unreadableObjects` —— 那不是关于
24+
数据源的证据;报告改用新增的 `DanglingReferenceReport.aborted` 记录「这次没跑完」,所以不完整
25+
依然是响的,只是不再占用发现桶。
26+
27+
**真正读不出来的对象(数据源故障)照旧进 `unreadableObjects`**,巡检在 CLI 场景也照旧运行 ——
28+
这里没有「一次性命令不跑巡检」的开关,只有「引擎活着才读」的生命周期边界。
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#4747] A one-shot CLI stack tears down through the KERNEL, and the ADR-0057
5+
* sweep stops with it.
6+
*
7+
* The bug this pins, end to end on the real `bootSchemaStack` path:
8+
*
9+
* $ os migrate recorded-by --json # exits 0, prints valid JSON …
10+
* ERROR Find operation failed {"object":"sys_metadata", …}
11+
* WARN [integrity] dangling-reference audit could not list an object …
12+
* WARN [integrity] stored references that resolve to nothing (#4551)
13+
* {"unreadableObjects":["sys_metadata","sys_view_definition"], …}
14+
*
15+
* Two silent no-ops stacked up to produce it. `shutdown()` called
16+
* `(runtime as any).stop?.()` and `Runtime` has no `stop`; the one thing that
17+
* would have disarmed the sweep was `ObjectQLPlugin.stop()`, a hook the kernel
18+
* never calls (the Plugin contract is `init`/`start`/`destroy`). So the kernel
19+
* stayed "running" with every timer armed while the command closed its driver,
20+
* and 60s later the sweep read a pool that was gone — filing both objects as
21+
* `unreadableObjects` on a completely healthy run.
22+
*
23+
* Two assertions matter here and they pull in opposite directions on purpose:
24+
*
25+
* - while the engine is LIVE, a CLI-booted stack really does audit (this is
26+
* not #4747's rejected option C — "one-shot commands skip the audit" would
27+
* make it permanently blind exactly where an operator has no other tool);
28+
* - once the stack is torn down, the sweep issues no reads at all.
29+
*/
30+
31+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
32+
import { mkdtempSync, writeFileSync, mkdirSync, rmSync } from 'node:fs';
33+
import { tmpdir } from 'node:os';
34+
import { join } from 'node:path';
35+
import { bootSchemaStack } from './schema-migrate.js';
36+
37+
interface LifecycleServiceLike {
38+
stopped: boolean;
39+
sweep(): Promise<{ danglingReferences?: { unreadableObjects: string[]; aborted?: boolean } }>;
40+
}
41+
42+
describe('[#4747] bootSchemaStack teardown disarms the ADR-0057 sweep', () => {
43+
let dir: string;
44+
let dbFile: string;
45+
const savedEnv: Record<string, string | undefined> = {};
46+
47+
beforeAll(() => {
48+
dir = mkdtempSync(join(tmpdir(), 'os-teardown-'));
49+
mkdirSync(join(dir, 'dist'), { recursive: true });
50+
mkdirSync(join(dir, 'data'), { recursive: true });
51+
dbFile = join(dir, 'data', 'app.db');
52+
writeFileSync(
53+
join(dir, 'dist', 'objectstack.json'),
54+
JSON.stringify({
55+
id: 'teardown_smoke',
56+
name: 'Teardown Smoke',
57+
objects: [
58+
{
59+
name: 'td_note',
60+
fields: {
61+
title: { type: 'text', required: true },
62+
// A real reference field, so the audit has something to read
63+
// rather than skipping the object outright.
64+
owner: { type: 'lookup', reference: 'td_person' },
65+
},
66+
},
67+
{ name: 'td_person', fields: { name: { type: 'text' } } },
68+
],
69+
}),
70+
);
71+
savedEnv.OS_ARTIFACT_PATH = process.env.OS_ARTIFACT_PATH;
72+
process.env.OS_ARTIFACT_PATH = join(dir, 'dist', 'objectstack.json');
73+
});
74+
75+
afterAll(() => {
76+
process.env.OS_ARTIFACT_PATH = savedEnv.OS_ARTIFACT_PATH;
77+
try { rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ }
78+
});
79+
80+
it('audits while the engine is live, and reads nothing once the stack is down', async () => {
81+
const stack = await bootSchemaStack({ databaseUrl: `file:${dbFile}`, projectRoot: dir });
82+
// Resolved BEFORE teardown — the point is what this same instance does
83+
// afterwards, and service resolution post-shutdown is not the subject.
84+
const lifecycle = stack.kernel.getService('lifecycle') as LifecycleServiceLike;
85+
expect(lifecycle).toBeTruthy();
86+
87+
// ── While the engine is live: the audit runs for real ─────────────────
88+
// Not "the CLI skips the audit" — it reads, and reports a clean, COMPLETE
89+
// run. An empty `unreadableObjects` here is a fact about the database,
90+
// which is precisely what it stopped being before this fix.
91+
expect(lifecycle.stopped).toBe(false);
92+
const live = await lifecycle.sweep();
93+
expect(live.danglingReferences).toBeDefined();
94+
expect(live.danglingReferences!.unreadableObjects).toEqual([]);
95+
expect(live.danglingReferences!.aborted).toBe(false);
96+
97+
// ── Teardown ──────────────────────────────────────────────────────────
98+
await stack.shutdown();
99+
100+
// The kernel really shut down. `(runtime as any).stop?.()` left it running
101+
// and every plugin undestroyed, which is how a missing teardown managed to
102+
// look exactly like a performed one.
103+
expect(stack.kernel.isRunning()).toBe(false);
104+
expect(lifecycle.stopped).toBe(true);
105+
106+
// The sweep the timer would have fired 60s later: no engine reads, so no
107+
// `ERROR Find operation failed` on a successful command, and no object
108+
// filed as unreadable for the crime of being asked after closing time.
109+
const afterDown = await lifecycle.sweep();
110+
expect(afterDown.danglingReferences).toBeUndefined();
111+
112+
// …and this is not vacuous: the pool really is closed, so a read issued
113+
// here really would fail. The silence above is the fix, not an absence of
114+
// anything to read.
115+
const engine = stack.kernel.getService('objectql') as {
116+
find(object: string, options: Record<string, unknown>): Promise<unknown[]>;
117+
};
118+
await expect(engine.find('td_note', { limit: 1, context: { isSystem: true } })).rejects.toThrow();
119+
}, 120_000);
120+
});

packages/cli/src/utils/schema-migrate.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,8 +209,28 @@ export async function bootSchemaStack(
209209
flushSchemaDdl: async () => (defer && driver?.flushDeferredSchemaDdl
210210
? await driver.flushDeferredSchemaDdl()
211211
: []),
212+
/**
213+
* Tear the one-shot stack down through the kernel's own teardown — the
214+
* same `kernel.shutdown()` `os serve` runs on SIGTERM, so a one-shot
215+
* command and a server take ONE path out (#4747).
216+
*
217+
* It used to call `(runtime as any).stop?.()`. `Runtime` has no `stop` —
218+
* the optional-call swallowed that fact, so every `os migrate` subcommand
219+
* closed its driver while leaving the kernel fully "running": no plugin
220+
* ever got `destroy()`, and the ADR-0057 lifecycle sweep stayed armed. 60s
221+
* later it woke inside the still-alive process and read through the pool
222+
* this line had already closed, which is why a successful command ended in
223+
* `ERROR Find operation failed` and a #4551 report naming `sys_metadata` /
224+
* `sys_view_definition` as unreadable. A cast plus `?.` is how a missing
225+
* teardown looks exactly like a performed one; there is no version of that
226+
* call that could ever have worked.
227+
*
228+
* The explicit `disconnect()` stays as the backstop for a driver this
229+
* kernel did not register through `DefaultDatasourcePlugin` (whose own
230+
* `destroy()` closes the ones it owns); a second disconnect is a no-op.
231+
*/
212232
shutdown: async () => {
213-
try { await (runtime as any).stop?.(); } catch { /* ignore */ }
233+
try { await kernel.shutdown(); } catch { /* teardown is best-effort */ }
214234
try { await driver?.disconnect?.(); } catch { /* ignore */ }
215235
},
216236
};

packages/objectql/src/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,9 @@ export type {
138138
DanglingReferenceAuditOptions,
139139
DanglingReferenceAuditPort,
140140
AuditableObject,
141+
// [#4747] The teardown bit the audit reads — exported alongside the options
142+
// type that names it, so a caller can spell the parameter it passes.
143+
AuditAbortSignal,
141144
} from './integrity/dangling-reference-audit.js';
142145

143146
// Export MetadataFacade

packages/objectql/src/integrity/dangling-reference-audit.test.ts

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,3 +372,162 @@ describe('[#4551] dangling stored references are reported, never rewritten', ()
372372
]);
373373
});
374374
});
375+
376+
/**
377+
* [#4747] "The datasource refused" and "nobody asked" are different facts.
378+
*
379+
* The audit reads through a live engine, and the engine outlives it only until
380+
* the host closes the pool. Before this, every `os migrate` invocation ended
381+
* with `unreadableObjects: ['sys_metadata', 'sys_view_definition']` — the sweep
382+
* fired 60s after boot, inside a process whose datasource had already been
383+
* disconnected. An `unreadableObjects` that is non-empty on every healthy run
384+
* is not a cautious report; it is a broken alarm, and it costs exactly the
385+
* signal #4551 built the bucket for.
386+
*
387+
* The pair of tests that opens this block is the whole point: BEFORE the fix
388+
* the two runs were indistinguishable, and after it they must never again be
389+
* confusable. Deleting the abort handling makes the second one fail; deleting
390+
* the `unreadableObjects` push makes the first one fail.
391+
*/
392+
describe('[#4747] a run that was called off is not a finding about the data', () => {
393+
/** A port whose listing always fails — the two runs below differ ONLY in why. */
394+
const unreadablePort = () => makePort({
395+
objects: [binding],
396+
rows: {},
397+
unreadable: new Set(['sys_position_permission_set']),
398+
});
399+
400+
it('a REAL datasource fault still lands in `unreadableObjects`, loudly', async () => {
401+
// The case the bucket exists for, and the one that must survive the fix:
402+
// the audit tried, the store would not answer, and the report must not read
403+
// as a clean bill of health on an object nothing could look at.
404+
const port = unreadablePort();
405+
406+
const out = await auditDanglingReferences(port);
407+
408+
expect(out.unreadableObjects).toEqual(['sys_position_permission_set']);
409+
expect(out.aborted).toBe(false);
410+
expect(port.warnings.map((w) => w[0])).toContain(
411+
'[integrity] dangling-reference audit could not list an object',
412+
);
413+
});
414+
415+
it('the SAME failure, raced by a teardown, is dropped instead of filed', async () => {
416+
// Identical throw from identical code — the only difference is that the
417+
// caller had called the run off, which is what closing a connection pool on
418+
// purpose looks like from in here. It is not evidence about the datasource,
419+
// so it must not spend the bucket that only holds evidence.
420+
const signal = { aborted: false };
421+
const port = unreadablePort();
422+
const failing = port.find.bind(port);
423+
port.find = async (o, opts) => {
424+
signal.aborted = true; // the pool closes mid-query
425+
return failing(o, opts); // …and the query fails because of it
426+
};
427+
428+
const out = await auditDanglingReferences(port, { signal });
429+
430+
expect(out.unreadableObjects).toEqual([]);
431+
// Not silence either: the run says it did not finish, so nothing can read
432+
// `dangling: []` as "everything is fine".
433+
expect(out.aborted).toBe(true);
434+
expect(port.warnings).toEqual([]);
435+
});
436+
437+
it('called off BEFORE a read: no query is issued at all', async () => {
438+
// This is what removes the `ERROR Find operation failed` line from a
439+
// SUCCESSFUL command — the engine never gets asked, so it never logs.
440+
const reads: string[] = [];
441+
const port = makePort({
442+
objects: [binding, task],
443+
rows: { sys_position_permission_set: [{ id: 'ppr_1', permission_set_id: 'ps_gone' }] },
444+
});
445+
const findSpy = port.find.bind(port);
446+
port.find = async (o, opts) => { reads.push(o); return findSpy(o, opts); };
447+
448+
const out = await auditDanglingReferences(port, { signal: { aborted: true } });
449+
450+
expect(reads).toEqual([]);
451+
expect(port.probes).toEqual([]);
452+
expect(out.aborted).toBe(true);
453+
expect(out.unreadableObjects).toEqual([]);
454+
});
455+
456+
it('called off MID-run keeps what it already proved and stops there', async () => {
457+
// A finding is a finding whenever it was made; only the SILENCE about the
458+
// rest of the run becomes unreliable, which is what `aborted` records. So
459+
// an abort must not discard the first object's verdict — and must not turn
460+
// the second object into a report about the datasource.
461+
const signal = { aborted: false };
462+
const reads: string[] = [];
463+
const port = makePort({
464+
objects: [binding, task], // binding is scanned first (security surface)
465+
rows: {
466+
sys_position_permission_set: [{ id: 'ppr_1', permission_set_id: 'ps_gone' }],
467+
showcase_task: [{ id: 't1', title: 'T', project: 'proj_gone' }],
468+
},
469+
unreadable: new Set(['showcase_task']),
470+
});
471+
const findSpy = port.find.bind(port);
472+
port.find = async (o, opts) => {
473+
reads.push(o);
474+
// Teardown lands as the second listing is issued — so that listing fails
475+
// BECAUSE the run was called off, which is the production race exactly.
476+
if (o === 'showcase_task') signal.aborted = true;
477+
return findSpy(o, opts);
478+
};
479+
480+
const out = await auditDanglingReferences(port, { signal });
481+
482+
expect(reads).toEqual(['sys_position_permission_set', 'showcase_task']);
483+
expect(out.dangling).toHaveLength(1);
484+
expect(out.dangling[0]).toMatchObject({ recordId: 'ppr_1', value: 'ps_gone' });
485+
expect(out.aborted).toBe(true);
486+
// The object the teardown cut short is NOT reported as unreadable.
487+
expect(out.unreadableObjects).toEqual([]);
488+
// The real finding is still reported, and the summary line carries the
489+
// incompleteness so the log cannot read as a finished run either.
490+
const summary = port.warnings.find((w) => w[0].includes('#4551'));
491+
expect(summary).toBeDefined();
492+
expect((summary![1] as any).aborted).toBe(true);
493+
});
494+
495+
it('a probe that fails under a teardown is not `undetermined` either', async () => {
496+
// `undetermined` means "the probe RAN and could not tell". A withdrawn
497+
// question did not run, so counting it there would repeat the same category
498+
// error one bucket over.
499+
const signal = { aborted: false };
500+
const port = makePort({
501+
objects: [binding],
502+
rows: { sys_position_permission_set: [{ id: 'ppr_1', permission_set_id: 'ps_x' }] },
503+
throwingTargets: new Set(['sys_permission_set']),
504+
});
505+
const probeSpy = port.probe.bind(port);
506+
port.probe = async (target, id) => {
507+
signal.aborted = true;
508+
return probeSpy(target, id);
509+
};
510+
511+
const out = await auditDanglingReferences(port, { signal });
512+
513+
expect(out.undetermined).toBe(0);
514+
expect(out.dangling).toEqual([]);
515+
expect(out.aborted).toBe(true);
516+
});
517+
518+
it('a run nobody called off says so explicitly — `aborted: false`, not absent', async () => {
519+
// The flag is a positive statement about completeness, so a consumer never
520+
// has to guess whether `undefined` meant "finished" or "old report shape".
521+
const port = makePort({
522+
objects: [binding],
523+
rows: { sys_position_permission_set: [{ id: 'ppr_1', permission_set_id: 'ps_real' }] },
524+
existing: new Set(['sys_permission_set ps_real']),
525+
});
526+
527+
const out = await auditDanglingReferences(port, { signal: { aborted: false } });
528+
expect(out.aborted).toBe(false);
529+
530+
const noSignal = await auditDanglingReferences(port);
531+
expect(noSignal.aborted).toBe(false);
532+
});
533+
});

0 commit comments

Comments
 (0)