From e3c28458b868b6e4f15efc5a1ec8ede338c01c35 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 03:33:14 +0000 Subject: [PATCH] test(triggers,approvals): pin expected read-refusal noise instead of muting it The two SqlDriver-backed fixtures of #11081 blanket-silenced their kernel, so the 109 expected authz/organization read refusals they provoke were suppressed along with every unexpected frame, and nothing asserted the expected reads still happen. Both now carry `@objectstack/runtime`'s shared expected-noise capture (#10629 / #10630): it withholds only a line naming a declared table AND carrying that same table's `no such table` reason, forwards every other driver fault to the real console, and counts what it withheld so `afterAll` can assert each channel fired. `logger: { level: 'silent' }` stays (#10984) -- it is the key ObjectKernelConfig reads, and it is what keeps ~1000 INFO/WARN frames out of the shared shard log. The import escapes the package on purpose, so it is DECLARED rather than left for CI to find: one file (the helper imports nothing) in CROSS_PACKAGE_TEST_INPUTS plus a matching `#test` inputs entry in turbo.json for each package. trigger-record-change's test program needs `rootDir` widened as a consequence, the same shape plugin-approvals already records for #10101. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015ahemw8RcTgqtxrj15PEZx --- .../status-mirror-cascade.integration.test.ts | 67 ++++++++++++++++- .../src/record-change-integration.test.ts | 72 ++++++++++++++++++- .../trigger-record-change/tsconfig.test.json | 24 +++++-- scripts/check-cross-package-test-inputs.mjs | 39 ++++++++++ turbo.json | 26 +++++++ 5 files changed, 221 insertions(+), 7 deletions(-) diff --git a/packages/plugins/plugin-approvals/src/status-mirror-cascade.integration.test.ts b/packages/plugins/plugin-approvals/src/status-mirror-cascade.integration.test.ts index e7c9e38b95..ec0ffc5352 100644 --- a/packages/plugins/plugin-approvals/src/status-mirror-cascade.integration.test.ts +++ b/packages/plugins/plugin-approvals/src/status-mirror-cascade.integration.test.ts @@ -36,7 +36,7 @@ * this file reads back all land in a real table. */ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, afterAll } from 'vitest'; import { ObjectKernel } from '@objectstack/core'; import { ObjectQLPlugin } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; @@ -46,9 +46,69 @@ import { ApprovalService } from './approval-service.js'; import { SysApprovalRequest } from './sys-approval-request.object.js'; import { SysApprovalAction } from './sys-approval-action.object.js'; import { SysApprovalApprover } from './sys-approval-approver.object.js'; +// [#11081] `@objectstack/runtime`'s shared expected-noise capture. This import +// escapes the package on PURPOSE, so it is DECLARED rather than left for CI to +// discover: `CROSS_PACKAGE_TEST_INPUTS` in +// `scripts/check-cross-package-test-inputs.mjs` names the one file, and +// `@objectstack/plugin-approvals#test` in `turbo.json` hashes the same path so +// the cache moves with it. The radius is that ONE file rather than +// `packages/runtime/src/**` (what `plugin-auth` and `dogfood` declare) because +// the helper imports nothing — it is the whole read. +import { captureExpectedReadRefusals } from '../../../runtime/src/expected-read-refusal-noise.js'; const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); +/** + * [#11081] The tables this fixture deliberately never provisions — and so the + * ONLY read refusals whose log frames may be withheld here. + * + * `beforeEach` boots a kernel with no datasource and attaches sqlite late, then + * syncs exactly four objects. Every approval decision below therefore probes + * the six authz tables `resolveUserAuthzGrants` reads plus + * `sys_approval_delegation`, which `ApprovalService.lookupActiveDelegation` + * reads best-effort on each decision. All are fail-soft, so the reads are + * EXPECTED — but the driver and the engine each log the fault on the way out. + * + * ⛔ MEASURED, not copied from the probers' source: a run of this file at + * `logger: { level: 'info' }` emitted 25 `refused a read on ''` driver lines + * and 25 matching `ERROR Find operation failed` engine frames — sys_user 5 / + * sys_member 4 / sys_position 4 / sys_user_position 4 / + * sys_user_permission_set 4 / sys_organization 2 / sys_approval_delegation 2. + * + * ## Why a capture instead of the blanket `silent` this replaces + * + * `logger: { level: 'silent' }` stays — it is the key `ObjectKernelConfig` + * actually reads, and it is what keeps this file's ~186 INFO and ~12 WARN + * frames out of the shared shard log. What it cannot do is tell the 25 expected + * refusals apart from a 26th that means something. The capture withholds only a + * line naming one of these tables AND carrying that same table's `no such + * table` reason, forwards every other driver fault to the real console, and + * COUNTS what it withheld so `afterAll` can assert the expected reads still + * happen. A capture nobody asserts is a mute. + */ +const EXPECTED_ABSENT_PROBE_TABLES = [ + 'sys_user', + 'sys_member', + 'sys_user_position', + 'sys_user_permission_set', + 'sys_position', + 'sys_organization', + 'sys_approval_delegation', +] as const; + +/** [#11081] Shared by both kernels this file boots; asserted once in `afterAll`. */ +const noise = captureExpectedReadRefusals([...EXPECTED_ABSENT_PROBE_TABLES]); + +/** + * [#11081] The PIN half. ⛔ Repairing a failure here means re-deriving the list + * above or finding out why a probe stopped firing — NEVER deleting the channel. + * In particular a silent `sys_approval_delegation` means the out-of-office + * delegation lookup stopped running on a decision, which is a finding. + */ +afterAll(() => { + expect(noise.silentChannels()).toEqual([]); +}); + const SUBMITTER = { userId: 'submitter', positions: [], permissions: [] } as any; const APPROVER = { userId: 'approver', positions: [], permissions: [] } as any; @@ -137,8 +197,13 @@ describe('an approval decision cascades as the deciding user (#3783)', () => { // The engine's own `init()` ran during bootstrap, before this driver // existed, so the connect the engine would have done is done here. const driver = makeSqliteDriver(); + // [#11081] Before `connect()` — i.e. before the driver runs any statement. + // The sink also RESTORES a loud channel: an unexpected driver fault reaches + // the real console from here even though the kernel logger is `silent`. + noise.captureDriver(driver); await driver.connect(); objectql.registerDriver(driver, true); + noise.captureEngine(objectql); for (const def of [opportunity, SysApprovalRequest, SysApprovalAction, SysApprovalApprover]) { objectql.registry.registerObject(def as any, 'approvals-test', 'approvals-test'); } diff --git a/packages/triggers/trigger-record-change/src/record-change-integration.test.ts b/packages/triggers/trigger-record-change/src/record-change-integration.test.ts index 2b805285d0..22e8413a8c 100644 --- a/packages/triggers/trigger-record-change/src/record-change-integration.test.ts +++ b/packages/triggers/trigger-record-change/src/record-change-integration.test.ts @@ -23,13 +23,22 @@ * that only ever held keys somebody set. */ -import { describe, it, expect, afterEach, vi } from 'vitest'; +import { describe, it, expect, afterEach, afterAll, vi } from 'vitest'; import { ObjectKernel } from '@objectstack/core'; import { ObjectQLPlugin } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; import { AutomationServicePlugin, type AutomationEngine } from '@objectstack/service-automation'; import type { IDataEngine, IObjectQLEngine } from '@objectstack/spec/contracts'; import { RecordChangeTriggerPlugin } from './plugin.js'; +// [#11081] `@objectstack/runtime`'s shared expected-noise capture. This import +// escapes the package on PURPOSE, so it is DECLARED rather than left for CI to +// discover: `CROSS_PACKAGE_TEST_INPUTS` in +// `scripts/check-cross-package-test-inputs.mjs` names the one file, and +// `@objectstack/trigger-record-change#test` in `turbo.json` hashes the same +// path so the cache moves with it. The radius is that ONE file rather than +// `packages/runtime/src/**` (which is what `plugin-auth` and `dogfood` +// declare) because the helper imports nothing — it is the whole read. +import { captureExpectedReadRefusals } from '../../../runtime/src/expected-read-refusal-noise.js'; /** * `check:slot-lookup` (#4251) — a NEW `kernel.getService(...)` site must carry @@ -55,6 +64,58 @@ type TestObjectQLEngine = IObjectQLEngine & { const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); +/** + * [#11081] The tables this file deliberately never provisions — and therefore + * the ONLY read refusals whose log frames may be withheld here. + * + * Every `it` below boots a kernel with no datasource, attaches sqlite late, and + * registers exactly its own test object. The authz resolver + * (`core/src/security/resolve-authz-context.ts`) and + * `ObjectQL.probeInstallOrganizations` then probe these six on every write. + * Both probes are fail-soft by construction, so the reads are EXPECTED — but + * the driver and the engine each log the fault on the way out. + * + * ⛔ This list is MEASURED, not copied from the probers' source: a run of this + * file at `logger: { level: 'info' }` emitted 84 `refused a read on ''` + * driver lines and 84 matching `ERROR Find operation failed` engine frames, + * split sys_user 15 / sys_member 15 / sys_user_position 15 / + * sys_user_permission_set 15 / sys_position 15 / sys_organization 9. A read + * this file stops provoking therefore shows up as a CHANGED SET (red + * `silentChannels()`), not as a silence. + * + * ## Why a capture instead of the blanket `silent` this replaces + * + * `logger: { level: 'silent' }` stays — it is the key `ObjectKernelConfig` + * actually reads and it is what keeps this file's ~830 INFO and ~46 WARN + * frames out of the shared shard log. What it CANNOT do is distinguish the 84 + * expected refusals above from an 85th that means something. The capture can: + * it withholds only a line naming one of these tables AND carrying that same + * table's `no such table` reason, forwards every other driver fault to the + * real console, and COUNTS what it withheld so `afterAll` can assert the + * expected reads still happen. A capture nobody asserts is a mute. + */ +const EXPECTED_ABSENT_PROBE_TABLES = [ + 'sys_user', + 'sys_member', + 'sys_user_position', + 'sys_user_permission_set', + 'sys_position', + 'sys_organization', +] as const; + +/** [#11081] Shared by every kernel this file boots; asserted once in `afterAll`. */ +const noise = captureExpectedReadRefusals([...EXPECTED_ABSENT_PROBE_TABLES]); + +/** + * [#11081] The PIN half. ⛔ Repairing a failure here means re-deriving the list + * above or finding out why a probe stopped firing — NEVER deleting the channel: + * a runtime read that silently stopped happening is exactly the finding this + * assertion exists to make loud. + */ +afterAll(() => { + expect(noise.silentChannels()).toEqual([]); +}); + /** * The real backend: better-sqlite3 `:memory:` through `@objectstack/driver-sql`, * built the canonical way (`examples/app-crm`, `cli db clean`, PR #5715). The @@ -89,8 +150,17 @@ afterEach(async () => { */ async function attachSqlite(objectql: any): Promise { const driver = makeSqliteDriver(); + // [#11081] Before `connect()` — i.e. before the driver runs any statement, the + // discipline `captureExpectedReadRefusals` documents. `logger` is a protected + // field with a `console` default, so the sink also RESTORES a loud channel: + // an unexpected driver fault reaches the real console from here even though + // the kernel logger is `silent`. + noise.captureDriver(driver); await driver.connect(); objectql.registerDriver(driver, true); + // The engine exists only after `kernel.bootstrap()`, which every caller has + // already run; the probed reads all happen later, per write. + noise.captureEngine(objectql); openDrivers.push(driver); return driver; } diff --git a/packages/triggers/trigger-record-change/tsconfig.test.json b/packages/triggers/trigger-record-change/tsconfig.test.json index 30978f284e..1f37c32b5c 100644 --- a/packages/triggers/trigger-record-change/tsconfig.test.json +++ b/packages/triggers/trigger-record-change/tsconfig.test.json @@ -48,10 +48,23 @@ // relative imports, and the whole layer compiles at 0 under the inherited // settings. Matching the program the ledger's 0 was measured over is the // point -- a different `module` would be a different measurement. -// - `rootDir` IS UNTOUCHED, also unlike those siblings. It stays `./src` -// because every test file is already under it: the re-measure reported -// TS6059 x0, so there is no misconfiguration diagnostic to neutralise and -// widening would only loosen which files may enter the program. +// - `rootDir` IS WIDENED to `../../..` (= `packages/`), which this file's +// first version deliberately did NOT do. That version's reason was a +// measurement -- "every test file is already under `./src`, the re-measure +// reported TS6059 x0" -- and [#11081] moved the measurement rather than the +// rule: `src/record-change-integration.test.ts` now imports +// `packages/runtime/src/expected-read-refusal-noise.ts` (the shared +// expected-noise capture, declared in +// `scripts/check-cross-package-test-inputs.mjs`), so the program contains a +// file outside `./src` and `tsc` reports TS6059 x1 against it. `rootDir` is +// enforced over every program file even under `--noEmit`, so the widening +// is the same consequence-of-an-import that `packages/plugins/ +// plugin-approvals` records for #10101 and `packages/rest` for #9960 -- +// `../../..` is the directory that genuinely contains every program file. +// ⛔ It buys nothing else: EMIT is unaffected (this config asserts `noEmit` +// and the tsup build reads `tsconfig.json`, whose entry is `src/index.ts` +// alone), `include` still names `src/**/*` so no new file may ENTER the +// program on its own, and STRICTNESS below is untouched. // - STRICTNESS IS UNTOUCHED. `strict`, `noUnusedLocals`, `noUnusedParameters`, // `noImplicitReturns` and the rest are inherited from the root config. // Nothing here may loosen a type rule; if a test does not compile, that is @@ -67,7 +80,8 @@ { "extends": "./tsconfig.json", "compilerOptions": { - "noEmit": true + "noEmit": true, + "rootDir": "../../.." }, "include": ["src/**/*"], "exclude": ["node_modules", "dist"] diff --git a/scripts/check-cross-package-test-inputs.mjs b/scripts/check-cross-package-test-inputs.mjs index f96c6291fe..7ab02833b1 100644 --- a/scripts/check-cross-package-test-inputs.mjs +++ b/scripts/check-cross-package-test-inputs.mjs @@ -617,6 +617,45 @@ export const CROSS_PACKAGE_TEST_INPUTS = { // high-privilege table, and cross-checks spec's own delegatable pin. globs: ['packages/spec/src/security/**', 'packages/spec/src/identity/**'], }, + '@objectstack/trigger-record-change': { + // [#11081] src/record-change-integration.test.ts imports + // `@objectstack/runtime`'s shared expected-noise capture so its 84 expected + // authz/organization read refusals are WITHHELD-AND-ASSERTED rather than + // blanket-muted by `logger: { level: 'silent' }`. + // + // ONE file, not `packages/runtime/src/**` (which is the radius + // `plugin-auth` and `dogfood` carry): the helper has no imports of its own, + // so that single path IS the whole escaping read. The narrow radius keeps + // this package's suite off every runtime diff while still moving the + // `#test` hash when the predicate it depends on changes. + // + // The three below are NAMED in this package's prose rather than read by it + // — `slot-lookup-baseline.json` and `kernel.ts` by comments that predate + // this entry, `check-cross-package-test-inputs.mjs` by the import comment + // added with it. Same shape as `check-nul-bytes.mjs` on the + // `@objectstack/cli` entry above, and settled the same way: the literal + // collector takes quoted paths without parsing, so a mention forces a + // declaration, and declaring the file is cheaper than rewording prose to + // dodge the scanner. (They were invisible until now only because a package + // with NO escaping test is never rostered at all.) + globs: [ + 'packages/runtime/src/expected-read-refusal-noise.ts', + 'scripts/check-cross-package-test-inputs.mjs', + 'scripts/slot-lookup-baseline.json', + 'packages/core/src/kernel.ts', + ], + }, + '@objectstack/plugin-approvals': { + // [#11081] src/status-mirror-cascade.integration.test.ts imports the same + // capture for its 25 expected refusals (the six authz tables plus + // `sys_approval_delegation`). Same one-file radius, same reason — plus this + // gate's own path, named in that file's import comment (see the note on the + // sibling entry above for why a mention is declared rather than reworded). + globs: [ + 'packages/runtime/src/expected-read-refusal-noise.ts', + 'scripts/check-cross-package-test-inputs.mjs', + ], + }, '@objectstack/dogfood': { // test/*-conformance.test.ts read a fixed roster of probe files across // runtime, rest, plugins and services by repo-relative path. Narrow to the diff --git a/turbo.json b/turbo.json index d01c071772..9b3244041e 100644 --- a/turbo.json +++ b/turbo.json @@ -154,6 +154,32 @@ "$TURBO_ROOT$/packages/types/src/node-isolation.test.ts" ] }, + "@objectstack/trigger-record-change#test": { + "dependsOn": ["^build"], + "inputs": [ + "$TURBO_DEFAULT$", + "!dist/**", + "!coverage/**", + "!.turbo/**", + "$TURBO_ROOT$/packages/runtime/src/expected-read-refusal-noise.ts", + "$TURBO_ROOT$/scripts/check-cross-package-test-inputs.mjs", + "$TURBO_ROOT$/scripts/slot-lookup-baseline.json", + "$TURBO_ROOT$/packages/core/src/kernel.ts" + ], + "outputs": [] + }, + "@objectstack/plugin-approvals#test": { + "dependsOn": ["^build"], + "inputs": [ + "$TURBO_DEFAULT$", + "!dist/**", + "!coverage/**", + "!.turbo/**", + "$TURBO_ROOT$/packages/runtime/src/expected-read-refusal-noise.ts", + "$TURBO_ROOT$/scripts/check-cross-package-test-inputs.mjs" + ], + "outputs": [] + }, "@objectstack/plugin-security#test": { "dependsOn": ["^build"], "outputs": [],