Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .github/workflows/lint.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -1834,6 +1834,33 @@ jobs:
- name: Test-source alias gate
run: pnpm check:test-source-alias

# The process-global ref'd-timer probe lives in ONE module (#10785). A
# `'Timeout'` count derived from `getActiveResourcesInfo()` reports the
# WHOLE process, so comparing two readings across an `await` scores a
# subject against a number the test does not own — every co-tenant file's
# timers and the runner's own non-unref'd 100ms throttle. The shape was
# found FIVE times over four cards and repaired at the site each time;
# twice the first signal was a shard-only intermittent red (once in CI,
# `expected 2 to be 4`; once in the MERGE QUEUE, where it stalls every
# lane at once) whose message names a timer count rather than the change
# that caused it. Nothing prevented the sixth.
# ⛔ The rule is deliberately NOT "is there an `await` between these two
# readings" — that is an AST question, and a text scan that answers it
# approximately fails SILENTLY on spellings it does not know, which is the
# failure mode AGENTS.md already records for source-scanning gates. This
# bans the RAW PROBE outside the one approved module instead: a
# grep-level question, a one-entry allowlist, and a RED default that names
# the file, the line and the import to write. Matched by IDENTIFIER rather
# than by `process.`-receiver, so a destructure or a `node:process` named
# import cannot slip past; comments are masked, so the docblocks that must
# NAME the probe to explain the rule do not have to dodge the scanner.
# A clean run also asserts the approved module is present and still reads
# the probe — otherwise "nobody reads it" would be true of a tree where
# the instrument was deleted.
# Static scan of ~4.8k sources; sub-second.
- name: Ref'd-timer probe containment
run: pnpm check:refd-timer-probe

# The TYPE axis of the same invariant (#8180). The gate above reads
# `vitest.config.*` and nothing else, so the identical exposure on the
# type axis was unguarded repo-wide — and its symptom is likewise a GREEN
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -103,6 +103,7 @@
"check:examples-live-imports": "node scripts/check-examples-live-imports.mjs --self-test && node scripts/check-examples-live-imports.mjs",
"examples:live-imports": "node scripts/check-examples-live-imports.mjs --list",
"check:test-source-alias": "node scripts/check-test-source-alias.mjs --self-test && node scripts/check-test-source-alias.mjs",
"check:refd-timer-probe": "node scripts/check-refd-timer-probe.mjs --self-test && node scripts/check-refd-timer-probe.mjs",
"check:type-source-resolution": "node scripts/check-type-source-resolution.mjs --self-test && node scripts/check-type-source-resolution.mjs",
"check:published-files": "node scripts/check-published-files.mjs --self-test && node scripts/check-published-files.mjs",
"check:published-readme-exports": "node scripts/check-published-readme-exports.mjs --self-test && node scripts/check-published-readme-exports.mjs",
Expand Down
1 change: 1 addition & 0 deletions packages/core/package.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -25,6 +25,7 @@
},
"devDependencies": {
"@objectstack/metadata-core": "workspace:*",
"@objectstack/refd-timer-testkit": "workspace:*",
"@types/node": "^26.2.0",
"esbuild": "^0.28.2",
"typescript": "^6.0.3",
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/health-monitor.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,7 +7,7 @@ import {
recordGuards,
refdTimeouts,
stillPinningTheLoop,
} from './refd-timer-probe.testkit.js';
} from '@objectstack/refd-timer-testkit';

describe('PluginHealthMonitor', () => {
let monitor: PluginHealthMonitor;
Expand DownExpand Up@@ -123,7 +123,7 @@ describe('PluginHealthMonitor', () => {

/**
* The instrument these pins measure with lives in
* `refd-timer-probe.testkit.ts`, together with the argument for its shape:
* `@objectstack/refd-timer-testkit`, together with the argument for its shape:
* `getActiveResourcesInfo()` is PROCESS-global, so two readings are only
* comparable across a window that crosses no event-loop turn — the very
* property this suite states below and the one three sibling pins were
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/hot-reload.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,7 +5,7 @@ import { HotReloadManager } from './hot-reload.js';
import type { ObjectLogger } from './logger.js';
import type { Plugin } from './types.js';
import type { HotReloadConfigParsed } from '@objectstack/spec/kernel';
import { recordGuards, stillPinningTheLoop } from './refd-timer-probe.testkit.js';
import { recordGuards, stillPinningTheLoop } from '@objectstack/refd-timer-testkit';

/** Records `error` reports; every other level is dropped. `child()` is self. */
function createRecordingLogger(errors: { message: string; error?: unknown }[]): ObjectLogger {
Expand DownExpand Up@@ -73,7 +73,7 @@ describe('HotReloadManager', () => {
manager.registerPlugin('guarded-plugin', config);

// The guard is named by the timeout it was armed with rather than
// counted out of the process — `refd-timer-probe.testkit.ts` explains
// counted out of the process — `@objectstack/refd-timer-testkit` explains
// why `reloadPlugin()`'s `await` makes an absolute count unsound (#10685).
let reloaded = false;
const guards = await recordGuards(config.shutdownTimeout, async () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/kernel.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import { ObjectKernel } from './kernel';
import { ServiceLifecycle, PluginMetadata } from './plugin-loader';
import type { Plugin, PluginContext } from './types';
import { recordGuards, stillPinningTheLoop } from './refd-timer-probe.testkit.js';
import { recordGuards, stillPinningTheLoop } from '@objectstack/refd-timer-testkit';

describe('ObjectKernel', () => {
let kernel: ObjectKernel;
Expand DownExpand Up@@ -243,7 +243,7 @@ describe('ObjectKernel', () => {
/**
* The real value that hung one-shot CLI processes: ObjectQLPlugin's.
* It is also what tells these guards apart from every other timer on
* the shared loop — see `refd-timer-probe.testkit.ts`, which explains
* the shared loop — see `@objectstack/refd-timer-testkit`, which explains
* why these pins name their guards instead of counting the process
* (#10685).
*/
Expand Down
108 changes: 77 additions & 31 deletions packages/core/src/timeout-guard.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,6 +2,11 @@

import { describe, it, expect, afterEach, vi } from 'vitest';
import { TimeoutGuard, raceWithTimeout } from './timeout-guard.js';
import {
recordGuards,
refdTimeouts,
stillPinningTheLoop,
} from '@objectstack/refd-timer-testkit';

/**
* A guard has two reclaimable halves — the TIMER and the PROMISE — and the
Expand All@@ -14,9 +19,6 @@ describe('TimeoutGuard (#4813, #10604)', () => {
vi.useRealTimers();
});

const refdTimers = () =>
process.getActiveResourcesInfo().filter((r) => r === 'Timeout').length;

/**
* Whether `expiry` has SETTLED, decided without hanging the suite: a
* settled promise runs its reaction in the current microtask drain, which
Expand DownExpand Up@@ -85,13 +87,19 @@ describe('TimeoutGuard (#4813, #10604)', () => {
// loop, Node exits before an unref'd guard can report the timeout
// (#4813). A leak-free kernel that no longer enforces its timeouts
// is strictly worse than the leak.
const before = refdTimers();
// `refdTimeouts()` is the RAW, process-global reading, and it is
// sound here for the one reason it is ever sound: this whole window
// is synchronous. No `await` sits between the samples, so no timer
// callback can run between them and the ambient value cancels
// exactly. Adding an `await` anywhere below silently invalidates it
// — that is when `recordGuards`/`stillPinningTheLoop` is the answer.
const before = refdTimeouts();
const guard = new TimeoutGuard(120_000, () => new Error('must not fire'));

expect(refdTimers()).toBe(before + 1);
expect(refdTimeouts()).toBe(before + 1);

guard.reclaim();
expect(refdTimers()).toBe(before);
expect(refdTimeouts()).toBe(before);
});

it('builds the timeout error only when the guard actually fires', async () => {
Expand DownExpand Up@@ -139,42 +147,80 @@ describe('raceWithTimeout (#10604)', () => {
});

/**
* ⚠️ `process.getActiveResourcesInfo()` is PROCESS-wide, and in CI this file
* ⚠️ Why this no longer reads the process-global count at all (#10785).
*
* `process.getActiveResourcesInfo()` is PROCESS-wide, and in CI this file
* shares one worker with three dozen others — so the absolute count is
* AMBIENT and this test does not own it. Foreign timers alive at the sample
* expire while this test awaits, pulling the reading DOWN: the shard read
* `expected 2 to be 4` on the third leg (#10661), the only one that spends
* `expected 2 to be 4` on the third leg (#10604), the only one that spends
* real time on the loop (the first two settle on microtasks, where no timer
* phase can run and the reading cannot move under the test).
*
* A leak is a GROWTH, so the assertion is about the direction the SUBJECT
* can move the count, never the ambient value. Non-increase keeps every bit
* of the detection — an unreclaimed 120s guard is `+1` here, which is what
* ablating the `clearTimeout` half reds — and gives up only the decrease,
* which nothing `raceWithTimeout` does can cause. Each leg re-anchors on its
* own sample so a comparison spans one outcome instead of the whole test.
* #10661 repaired that by asserting NON-INCREASE, on the reasoning that a
* leak is a GROWTH so the direction that matters survives while foreign
* expiry stops being a signal. That decision was deliberate and correct as
* far as it went — and it is superseded here rather than reverted, because
* the reading it kept is still the whole process, and the arithmetic
* composes the wrong way on the one leg that spends real time on the loop:
*
* subject leaks +1
* two foreign timers expire -2
* reading before - 1 => toBeLessThanOrEqual PASSES
*
* A real leak, green. Nothing was ever red from this — the ablation cited
* on #10661 does red, run without ambient noise — but under shard load the
* detection is probabilistic rather than certain, which is a narrower
* guarantee than the comment above it claimed.
*
* The instrument removes the ambience instead of choosing a direction that
* survives it: `recordGuards` captures the `Timeout` handles THIS subject
* arms, told apart from every other timer on the shared loop by the very
* timeout they were configured with, and `stillPinningTheLoop` reports how
* many of those are still holding the loop open across a window that is
* synchronous by construction. So the assertion is `toBe(0)` again — an
* exact count of the subject's OWN leak, immune to foreign expiry in either
* direction, on all three legs including the one that burns 10ms of real
* time. `@objectstack/refd-timer-testkit` carries the full argument.
*
* ⚠️ The third leg's guard is SUPPOSED to fire — that is how the race is
* decided — so what it pins is different from the first two and is stated
* rather than implied: exactly one guard is armed for the losing race, and a
* fired guard reclaims itself (`stillPinningTheLoop` counts what a
* `clearTimeout` can still take away, and a timer that already ran is gone
* either way). The `clearTimeout`-half leak is detected by the first two
* legs, whose 120s guards are `+1` there when `reclaim()` stops clearing.
*
* ⛔ Do not "fix" the ambience by isolating this file or pinning the shard
* layout: that would make the pin's validity a property of the runner
* config rather than of its own assertion.
*/
it('leaves no ref\'d timer behind on any of the three outcomes', async () => {
const refd = () => process.getActiveResourcesInfo().filter((r) => r === 'Timeout').length;
let before = refd();

await raceWithTimeout(Promise.resolve('ok'), 120_000, () => new Error('must not fire'));
expect(refd()).toBeLessThanOrEqual(before);

before = refd();
await expect(
raceWithTimeout(Promise.reject(new Error('x')), 120_000, () => new Error('must not fire')),
).rejects.toThrow('x');
expect(refd()).toBeLessThanOrEqual(before);

before = refd();
await expect(
raceWithTimeout(new Promise(() => {}), 10, () => new Error('hung')),
).rejects.toThrow('hung');
expect(refd()).toBeLessThanOrEqual(before);
// Leg 1 — the operation wins. Each leg re-anchors on its own recording,
// so a verdict spans one outcome instead of the whole test.
const won = await recordGuards(120_000, () =>
raceWithTimeout(Promise.resolve('ok'), 120_000, () => new Error('must not fire')),
);
expect(won).toHaveLength(1);
expect(stillPinningTheLoop(won)).toBe(0);

// Leg 2 — the operation rejects.
const threw = await recordGuards(120_000, async () => {
await expect(
raceWithTimeout(Promise.reject(new Error('x')), 120_000, () => new Error('must not fire')),
).rejects.toThrow('x');
});
expect(threw).toHaveLength(1);
expect(stillPinningTheLoop(threw)).toBe(0);

// Leg 3 — the guard wins, after 10ms of real time on the loop. The leg
// #10661 had to weaken; it needs no weakening now.
const fired = await recordGuards(10, async () => {
await expect(
raceWithTimeout(new Promise(() => {}), 10, () => new Error('hung')),
).rejects.toThrow('hung');
});
expect(fired).toHaveLength(1);
expect(stillPinningTheLoop(fired)).toBe(0);
});
});
33 changes: 33 additions & 0 deletions packages/qa/refd-timer-testkit/package.json
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
{
"name": "@objectstack/refd-timer-testkit",
"version": "0.1.0",
"private": true,
"license": "Apache-2.0",
"description": "The subject-scoped instrument the ref'd-timer leak pins measure with (#4813, #6329, #10604, #10685, #10783). A process-global `getActiveResourcesInfo()` timer count is ambient — it belongs to every co-tenant test file in the worker — so this records the handles the SUBJECT arms and asks how many of those still pin the loop, across a window that is synchronous by construction. Not published; the one home `pnpm check:refd-timer-probe` allows the raw probe to be read from.",
"type": "module",
"exports": {
".": "./src/index.ts"
},
"scripts": {
"typecheck": "tsc --noEmit"
},
"devDependencies": {
"@types/node": "^26.2.0",
"typescript": "^6.0.3"
},
"keywords": [
"objectstack",
"qa",
"testkit",
"event-loop",
"timers"
],
"author": "ObjectStack",
"repository": {
"type": "git",
"url": "https://github.com/objectstack-ai/objectstack.git",
"directory": "packages/qa/refd-timer-testkit"
},
"homepage": "https://objectstack.ai/docs",
"bugs": "https://github.com/objectstack-ai/objectstack/issues"
}
Original file line numberDiff line numberDiff line change
Expand Up@@ -61,9 +61,51 @@
* instead: that would make a pin's validity a property of the runner config
* rather than of its own assertion.
*
* `.testkit.ts`, not `.test.ts`: it holds no assertions and must not be
* collected as a suite. It is imported only by tests, so tsup (entries
* `src/index.ts` and `src/logger.ts`) never bundles it into `dist`.
* ## This module is the ONLY place the raw probe may be read (#10785)
*
* `pnpm check:refd-timer-probe` enforces that repo-wide, by identifier and with
* comments masked, and its allowlist is this one file. The rule is deliberately
* the cheap one — ban the raw probe outside one approved module — rather than
* the precise-sounding "no `await` between two readings", which is an AST
* question that a text scan answers approximately and therefore SILENTLY; the
* gate's own header carries that argument and the five-instance table behind it.
*
* ⛔ Do not fork a copy of this file into another package. A second copy leaves
* two instruments with one argument between them, which is how the argument
* gets lost.
*
* ## Why it lives in a private package of its own (#10783)
*
* It sat in `packages/core/src/` until a FIFTH site turned up in
* `@objectstack/service-automation` and could not reach it — `@objectstack/core`
* exports exactly `.` and `./logger`. The three ways out were MEASURED rather
* than argued:
*
* - A test-only RELATIVE import into `packages/core/src` puts a foreign file
* into the consumer's tsc program, and that consumer's `rootDir` rejects it:
* `TS6059`, counted against `@objectstack/service-automation`'s frozen
* type-check debt (measured 3 → 4), a ledger that is shrink-only with no
* tolerance band and whose only other remedy is ⛔ MAINTAINER-ONLY. Most
* packages here set `rootDir: src`, so this was a trap for the next adopter
* too, not a quirk of one manifest.
* - Publishing it on `@objectstack/core`'s `exports` map would put a test
* helper on a runtime package's PUBLIC npm surface, under semver, for every
* consumer of the kernel — capability expansion with no external pull.
* - A bare specifier resolved through `node_modules` is exempt from `rootDir`
* (measured, with `--noEmit` and with emit), so a private workspace package
* is reachable from any consumer whatever its own build config says.
*
* Hence: `private: true`, no build step, `exports` straight at `src` — there is
* no `dist` for a consumer to go stale against, which is the exposure
* `check:test-source-alias` exists for. Every consumer takes it as a
* `devDependency`, and turbo hashes it through that edge rather than through a
* hand-written input glob: measured on `turbo run test --dry=json`, editing this
* file moves `@objectstack/service-automation#test`'s hash
* (`ee8253e9cb374faa` → `77680cb1d4bdd2d4`), so the cache cannot replay a green
* over a changed instrument.
*
* `src/index.ts` holding no assertions is load-bearing: it must never be
* collected as a suite, and this package declares no `test` script at all.
*
* ⚠️ Real timers only. Under `vi.useFakeTimers()` the handles are fakes and
* `getActiveResourcesInfo()` cannot see them — use `vi.getTimerCount()` there,
Expand Down
17 changes: 17 additions & 0 deletions packages/qa/refd-timer-testkit/tsconfig.json
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
{
"extends": "../../../tsconfig.json",
"compilerOptions": {
"outDir": "./dist",
"rootDir": "./src",
"types": [
"node"
]
},
"include": [
"src/**/*"
],
"exclude": [
"node_modules",
"dist"
]
}
1 change: 1 addition & 0 deletions packages/services/service-automation/package.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -27,6 +27,7 @@
"@objectstack/metadata-core": "workspace:*",
"@objectstack/objectql": "workspace:*",
"@objectstack/plugin-security": "workspace:*",
"@objectstack/refd-timer-testkit": "workspace:*",
"@objectstack/service-job": "workspace:*",
"@objectstack/service-messaging": "workspace:*",
"@types/node": "^26.2.0",
Expand Down
Loading
Loading