Skip to content

Commit 4e71ae1

Browse files
os-zhuangclaude
andauthored
fix(objectql): surface a failed lifecycle governance row-count probe instead of skipping the object (#8906) (#9105)
Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0150322 commit 4e71ae1

3 files changed

Lines changed: 292 additions & 1 deletion

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/objectql': patch
3+
---
4+
5+
lifecycle: a failed governance row-count probe is no longer indistinguishable from a quiet object
6+
7+
`LifecycleService.checkGovernance()` probed each declared object's row count and swallowed
8+
every failure with a bare `catch { continue }`. A driver outage therefore read exactly like
9+
an object with nothing to alert on: no `quota-exceeded`, no `growth`, nothing logged, and
10+
nothing in the sweep report — and because the failed object also dropped out of the count
11+
map that becomes the next sweep's baseline, the next sweep could not alert on growth for it
12+
either.
13+
14+
The probe now discriminates by error type through the shared `isMissingTableError`
15+
predicate. An unprovisioned table is truthful emptiness and stays silent; every other
16+
failure is reported per object in the sweep report's existing `errors` list and logged at
17+
`warn`, both naming the lost growth baseline. No new report field, no new error code, and
18+
the sweep is still isolated — one object's failed probe never costs the others their
19+
governance.

packages/objectql/src/lifecycle/lifecycle-service.test.ts

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1095,6 +1095,220 @@ describe('LifecycleService.sweep — governance (P4)', () => {
10951095
});
10961096
});
10971097

1098+
// #8906 — the governance row-count probe used to fail into `catch { continue }`,
1099+
// which made "this object has nothing worth alerting on" and "the driver could
1100+
// not answer" the same observable event. The damage outlived the sweep: no
1101+
// `quota-exceeded`, no `growth`, AND the object dropped out of `nextCounts`, so
1102+
// the next sweep had no baseline to diff against either.
1103+
//
1104+
// The repair discriminates by error TYPE through the declared
1105+
// `isMissingTableError` predicate and surfaces everything else through the
1106+
// channels that already exist — `report.errors` plus a `warn` — with no new
1107+
// report field (the maintainer's 2026-08-15 disposition for this family:
1108+
// unprovisioned is truthful emptiness, everything else must surface).
1109+
//
1110+
// Every expectation below is written as a LITERAL, never derived from the code
1111+
// under test, and the benign case asserts that the injected throw really fired —
1112+
// otherwise "the sweep carried on" would also be satisfied by a harness that
1113+
// never probed at all.
1114+
describe('LifecycleService.sweep — governance row-count probe failure (#8906)', () => {
1115+
const PROBED: LifecycleObjectLike = {
1116+
name: 'sys_job_run',
1117+
lifecycle: { class: 'telemetry', retention: { maxAge: '30d' } } as any,
1118+
};
1119+
/** A second declared object, so "one probe failed" can be told apart from
1120+
* "the governance leg stopped". */
1121+
const SIBLING: LifecycleObjectLike = {
1122+
name: 'sys_audit_log',
1123+
lifecycle: { class: 'telemetry', retention: { maxAge: '30d' } } as any,
1124+
};
1125+
1126+
function fakeSettings(values: Record<string, unknown>) {
1127+
return {
1128+
async get(_ns: string, key: string) {
1129+
if (key in values) return { value: values[key], source: 'global' };
1130+
return { value: undefined, source: 'default' };
1131+
},
1132+
};
1133+
}
1134+
1135+
/** A driver whose `count` answers per object — throwing for the objects named
1136+
* in `throwsFor`, and returning `rows` for every other one. */
1137+
function countingDriver(throwsFor: Record<string, unknown>, rows = 1_500) {
1138+
const count = vi.fn(async (object: string) => {
1139+
if (object in throwsFor) throw throwsFor[object];
1140+
return rows;
1141+
});
1142+
return { driver: { name: 'default', count }, count };
1143+
}
1144+
1145+
/** POSITIVE CONTROL — the same harness, nothing injected. Without this, every
1146+
* refusal below is also consistent with a fixture that never alerts at all. */
1147+
it('a healthy probe alerts on the quota and reports no error', async () => {
1148+
const warn = vi.fn();
1149+
const { driver, count } = countingDriver({});
1150+
const { engine } = captureEngine([PROBED], { driver });
1151+
const settings = fakeSettings({ quotas: { sys_job_run: 1_000 } });
1152+
1153+
const report = await service(engine, {
1154+
getSettings: () => settings,
1155+
logger: { ...silentLogger(), warn },
1156+
// Alerts go to the sink, so `warn` carries only degradation reports —
1157+
// an alert of its own logs at `warn` when no sink is registered.
1158+
onAlert: () => {},
1159+
}).sweep();
1160+
1161+
expect(count).toHaveBeenCalledWith('sys_job_run');
1162+
expect(report.alerts).toEqual([
1163+
{ type: 'quota-exceeded', object: 'sys_job_run', rowCount: 1_500, quota: 1_000 },
1164+
]);
1165+
expect(report.errors).toEqual([]);
1166+
expect(warn).not.toHaveBeenCalled();
1167+
});
1168+
1169+
it('a non-benign probe failure is reported per object, at warn, naming the lost baseline', async () => {
1170+
const warn = vi.fn();
1171+
const error = vi.fn();
1172+
const { driver } = countingDriver({ sys_job_run: new Error('connection reset by peer') });
1173+
const { engine } = captureEngine([PROBED], { driver });
1174+
const settings = fakeSettings({ quotas: { sys_job_run: 1 } });
1175+
1176+
const report = await service(engine, {
1177+
getSettings: () => settings,
1178+
logger: { ...silentLogger(), warn, error },
1179+
}).sweep();
1180+
1181+
// The report carries the incomplete fact in the field it already has —
1182+
// `errors` is the sweep's declared per-object failure channel, and the
1183+
// sweep's own summary line counts it. No new field was added.
1184+
expect(report.errors).toEqual([
1185+
{
1186+
object: 'sys_job_run',
1187+
error:
1188+
'governance row-count probe failed (connection reset by peer) — quota and growth alerting ' +
1189+
'skipped for this object this sweep, and its growth baseline for the next sweep is lost',
1190+
},
1191+
]);
1192+
// The damage is still real and still visible: quota 1 would have breached
1193+
// on any count at all, and no alert could be raised for an object nobody
1194+
// could count.
1195+
expect(report.alerts).toEqual([]);
1196+
expect(warn).toHaveBeenCalledWith(
1197+
'[lifecycle] governance row-count probe on sys_job_run failed (connection reset by peer); ' +
1198+
'quota/growth alerting skipped this sweep and the next sweep has no growth baseline for it',
1199+
);
1200+
// Reduced ALERTING, not a write that claimed to persist and did not —
1201+
// `warn`, deliberately, per AGENTS.md "Degradation log levels".
1202+
expect(error).not.toHaveBeenCalled();
1203+
});
1204+
1205+
it('one failed probe never costs the other objects their governance', async () => {
1206+
const { driver } = countingDriver({ sys_job_run: new Error('connection reset by peer') }, 2_000);
1207+
const { engine } = captureEngine([PROBED, SIBLING], { driver });
1208+
const settings = fakeSettings({ quotas: { sys_job_run: 1, sys_audit_log: 1_000 } });
1209+
1210+
const report = await service(engine, { getSettings: () => settings }).sweep();
1211+
1212+
expect(report.alerts).toEqual([
1213+
{ type: 'quota-exceeded', object: 'sys_audit_log', rowCount: 2_000, quota: 1_000 },
1214+
]);
1215+
expect(report.errors).toEqual([
1216+
{
1217+
object: 'sys_job_run',
1218+
error:
1219+
'governance row-count probe failed (connection reset by peer) — quota and growth alerting ' +
1220+
'skipped for this object this sweep, and its growth baseline for the next sweep is lost',
1221+
},
1222+
]);
1223+
});
1224+
1225+
// The report is where an operator learns that the degradation OUTLIVES the
1226+
// sweep it happened in. This pins that honestly: the fix makes the loss
1227+
// visible, it does not invent a baseline to replace it.
1228+
it('reports the lost baseline rather than repairing it — the next sweep still has no growth delta', async () => {
1229+
const outage = new Error('connection reset by peer');
1230+
const throwsFor: Record<string, unknown> = { sys_job_run: outage };
1231+
const { driver } = countingDriver(throwsFor, 5_000);
1232+
const { engine } = captureEngine([PROBED], { driver });
1233+
const settings = fakeSettings({ quotas: { sys_job_run: 100_000 }, growth_alert_rows: 100 });
1234+
const svc = service(engine, { getSettings: () => settings });
1235+
1236+
const first = await svc.sweep();
1237+
expect(first.errors).toHaveLength(1);
1238+
expect(first.errors[0].error).toContain('growth baseline for the next sweep is lost');
1239+
1240+
// The driver recovers. There is still no `last` to diff 5_000 against, so
1241+
// no growth alert can be raised — exactly what the first report said.
1242+
delete throwsFor.sys_job_run;
1243+
const second = await svc.sweep();
1244+
expect(second.errors).toEqual([]);
1245+
expect(second.alerts).toEqual([]);
1246+
1247+
// And the baseline is rebuilt from here: the sweep after that CAN alert.
1248+
const { driver: grown } = countingDriver({}, 6_000);
1249+
(engine as any).getDriverForObject = () => grown;
1250+
const third = await svc.sweep();
1251+
expect(third.alerts).toEqual([
1252+
{ type: 'growth', object: 'sys_job_run', rowCount: 6_000, delta: 1_000 },
1253+
]);
1254+
});
1255+
1256+
it('an unprovisioned table stays silent — truthful emptiness, not a swallowed outage', async () => {
1257+
const warn = vi.fn();
1258+
const missingTable = new Error('no such table: sys_job_run');
1259+
const { driver, count } = countingDriver({ sys_job_run: missingTable }, 2_000);
1260+
const { engine } = captureEngine([PROBED, SIBLING], { driver });
1261+
const settings = fakeSettings({ quotas: { sys_job_run: 1, sys_audit_log: 1_000 } });
1262+
1263+
const report = await service(engine, {
1264+
getSettings: () => settings,
1265+
logger: { ...silentLogger(), warn },
1266+
onAlert: () => {},
1267+
}).sweep();
1268+
1269+
// The injected throw ACTUALLY FIRED — without this, "the sweep continued"
1270+
// is also what a harness that never probed would produce.
1271+
expect(count).toHaveBeenCalledWith('sys_job_run');
1272+
await expect(count.mock.results[0]!.value).rejects.toThrow('no such table: sys_job_run');
1273+
1274+
// A table that does not exist holds no rows: no quota to breach, no growth
1275+
// to measure, nothing to report and nothing to log.
1276+
expect(report.errors).toEqual([]);
1277+
expect(warn).not.toHaveBeenCalled();
1278+
// …and the sweep carried on, proved by the sibling's alert.
1279+
expect(report.alerts).toEqual([
1280+
{ type: 'quota-exceeded', object: 'sys_audit_log', rowCount: 2_000, quota: 1_000 },
1281+
]);
1282+
});
1283+
1284+
// The benign verdict comes from the declared predicate, not from a substring
1285+
// guess: Postgres phrases a missing COLUMN on an existing relation as
1286+
// `column "x" of relation "y" does not exist`, which CONTAINS a legal
1287+
// missing-table phrase. The table is there and its rows were not counted, so
1288+
// this must surface.
1289+
it('a missing column on a provisioned table is not benign, though its message contains a table phrase', async () => {
1290+
const columnGone: Error & { code?: string } = new Error(
1291+
'column "status" of relation "sys_job_run" does not exist',
1292+
);
1293+
columnGone.code = '42703';
1294+
const { driver } = countingDriver({ sys_job_run: columnGone });
1295+
const { engine } = captureEngine([PROBED], { driver });
1296+
const settings = fakeSettings({ quotas: { sys_job_run: 1 } });
1297+
1298+
const report = await service(engine, { getSettings: () => settings }).sweep();
1299+
1300+
expect(report.errors).toEqual([
1301+
{
1302+
object: 'sys_job_run',
1303+
error:
1304+
'governance row-count probe failed (column "status" of relation "sys_job_run" does not exist) — ' +
1305+
'quota and growth alerting skipped for this object this sweep, and its growth baseline for the ' +
1306+
'next sweep is lost',
1307+
},
1308+
]);
1309+
});
1310+
});
1311+
10981312
// #5195 — ADR-0057 P4 lets an operator override any object's window through the
10991313
// `lifecycle` settings namespace, and until this the only validation on that
11001314
// override was "does it parse". That is a side door around #5179's invariant:

packages/objectql/src/lifecycle/lifecycle-service.ts

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import type { Lifecycle } from '@objectstack/spec/data';
44
import type { DriverQuery } from '@objectstack/spec/contracts';
5+
import { isMissingTableError } from '@objectstack/metadata/errors';
56
import { parseLifecycleDuration } from './duration.js';
67
import type {
78
DanglingReferenceAuditOptions,
@@ -802,7 +803,64 @@ export class LifecycleService {
802803
let rowCount: number;
803804
try {
804805
rowCount = await driver.count(obj.name);
805-
} catch {
806+
} catch (error) {
807+
// [#8906] A row-count probe that FAILED is not an object with nothing
808+
// to say. The bare `catch { continue }` this replaces made the two
809+
// indistinguishable, and the damage outlived the sweep it happened in:
810+
// the object is skipped for `quota-exceeded` AND for `growth` alerting
811+
// this sweep, and — because `nextCounts` is what becomes
812+
// `this.lastCounts` below — it is also dropped from the BASELINE, so
813+
// the next sweep has no `last` to diff against and cannot alert on
814+
// growth either. A driver outage read exactly like a quiet, healthy
815+
// object, twice over, with nothing logged and nothing in the report.
816+
//
817+
// Benign: the object is registered but its TABLE was never provisioned
818+
// (schema sync not run yet). It holds no rows, so there is no quota to
819+
// breach and no growth to measure — skipping it IS the truth. Asked
820+
// through the shared `isMissingTableError` predicate
821+
// (`@objectstack/metadata/errors`), never a hand-rolled code test, so
822+
// one vocabulary of "benign driver error" serves every seam that needs
823+
// one. It stays out of `nextCounts` deliberately: seeding a 0 baseline
824+
// for a table that does not exist would fire a phantom `growth` alert
825+
// on the first sweep after the table is provisioned and seeded.
826+
//
827+
// Everything else (connection drop, timeout, permission denial, a
828+
// dialect error) means the rows may well exist and simply were not
829+
// counted — the maintainer's 2026-08-15 disposition for this family:
830+
// unprovisioned is truthful emptiness, everything else must surface.
831+
//
832+
// It surfaces through the channels that already exist — no new report
833+
// field and no new error code. `report.errors` is this sweep's declared
834+
// per-object failure channel (the object loop in `sweep()` fills it the
835+
// same way) and the sweep's summary line already counts it; the `warn`
836+
// matches that loop's level, because the consequence is reduced
837+
// ALERTING, not a write that claimed to persist and did not
838+
// (AGENTS.md "Degradation log levels"). Both messages name the baseline
839+
// loss, since that is the half an operator cannot infer from a report
840+
// that is otherwise identical to a healthy one.
841+
//
842+
// Rethrowing — the shape #8895 took at the `cascadeDeleteRelations`
843+
// probe — is deliberately NOT the shape here, and not for uniformity's
844+
// sake: there, the caller is a `delete()` that must fail. Here the only
845+
// caller is `sweep()`, whose scheduler entry point is `void this.sweep()`
846+
// — a throw would land as an unhandled rejection, abandon governance for
847+
// every object still queued behind this one, skip `this.lastCounts =
848+
// nextCounts` entirely (losing EVERY object's baseline, not just this
849+
// one's), and break the documented invariant that a sweep failure is
850+
// isolated and never thrown into the scheduler. That is strictly more
851+
// damage than the defect being repaired.
852+
if (isMissingTableError(error)) continue;
853+
const msg = (error as Error)?.message ?? String(error);
854+
report.errors.push({
855+
object: obj.name,
856+
error:
857+
`governance row-count probe failed (${msg}) — quota and growth alerting skipped ` +
858+
`for this object this sweep, and its growth baseline for the next sweep is lost`,
859+
});
860+
this.opts.logger.warn(
861+
`[lifecycle] governance row-count probe on ${obj.name} failed (${msg}); ` +
862+
'quota/growth alerting skipped this sweep and the next sweep has no growth baseline for it',
863+
);
806864
continue;
807865
}
808866
nextCounts.set(obj.name, rowCount);

0 commit comments

Comments
 (0)