diff --git a/.changeset/localization-failure-memo-backend-leg-only.md b/.changeset/localization-failure-memo-backend-leg-only.md new file mode 100644 index 0000000000..ef9fb5b170 --- /dev/null +++ b/.changeset/localization-failure-memo-backend-leg-only.md @@ -0,0 +1,37 @@ +--- +"@objectstack/core": patch +--- + +fix(core): only a backend fault populates `resolveLocalizationContext`'s failure memo (#11877) + +`resolveLocalizationContext` memoizes an outcome for 30s whenever the read +"failed" (#10221 — so a repeatedly-failing `sys_setting` query does not re-run, +and the driver does not re-log it, on every request). The write condition was +wider than the cache's own docblock: six legs set the flag and only **one** of +them is the backend fault the docblock describes (the direct `ql.find` throw). +The other five are the **settings service refusing** — a thrown `getMany`, each +of the three older per-key `get`s, and the whole-block "service unavailable" +handler. + +Those five legs are reachable inside the settings engine's **bind window** +(`SettingsService.getMany` refuses all-or-nothing for a `localization` +namespace whose manifest is not yet registered), so: + +- A caller that deliberately re-reads **after** the bind — the #11580 stdio + repair re-resolves at `kernel:bootstrapped` for exactly this reason — was + answered from the memo taken **inside** the window for up to 30s. The + correction silently did not happen, with nothing in the output saying so. +- A settings refusal standing alongside a perfectly **successful** direct read + memoized that successful value — the staleness the docblock forbids outright + and that `analytics-timezone.dogfood.test.ts` (#1982/#2018) exists to catch. + +The memo is now written only for the direct-read fault. **#10221's protection +is unchanged for the legs it was built for**: its environment (table not +migrated yet) still memoizes, because the direct read throws there whether or +not a settings refusal stands in front of it — pinned in both directions. And +nothing is lost on the narrowed legs: those refusals throw out of an in-memory +registry check *before* any query and *before* any log line, so memoizing them +suppressed neither. + +No signature, export or accepted-input change — the flag is internal to the +module. diff --git a/packages/core/src/security/resolve-authz-context.test.ts b/packages/core/src/security/resolve-authz-context.test.ts index 23d041f355..3f1135a743 100644 --- a/packages/core/src/security/resolve-authz-context.test.ts +++ b/packages/core/src/security/resolve-authz-context.test.ts @@ -374,6 +374,22 @@ describe('resolveLocalizationContext — batched fallback read (#2409)', () => { }); }); +// Simulates a fresh environment: `sys_setting` not migrated/written yet, so +// every read rejects the way the real sql-driver's "no such table" does. +// Shared by the #10221 cache block below and the #11877 leg-narrowing block +// after it, which needs the same backend fault standing BEHIND a settings +// refusal. +function makeMissingTableQl() { + const counts = { sys_setting: 0 }; + return { + counts, + async find(_object: string) { + counts.sys_setting += 1; + throw new Error('no such table: sys_setting'); + }, + }; +} + // #10221: a fresh environment's `sys_setting` table doesn't exist yet, so // EVERY request's read used to fail and the sql-driver's `[sql-driver] // DATABASE_ERROR` warning repeated once per request, burying real errors. @@ -395,19 +411,6 @@ describe('resolveLocalizationContext — failure-only cross-request cache (#1022 vi.useRealTimers(); }); - // Simulates a fresh environment: `sys_setting` not migrated/written yet, so - // every read rejects the way the real sql-driver's "no such table" does. - function makeMissingTableQl() { - const counts = { sys_setting: 0 }; - return { - counts, - async find(_object: string) { - counts.sys_setting += 1; - throw new Error('no such table: sys_setting'); - }, - }; - } - it('does not re-query on a second call within the TTL window when the read fails (same tenant)', async () => { const ql = makeMissingTableQl(); const first = await resolveLocalizationContext({ ql, tenantId: 'o1' }); @@ -484,6 +487,193 @@ describe('resolveLocalizationContext — failure-only cross-request cache (#1022 }); }); +// ── #11877 — a SETTINGS-SERVICE refusal must not populate the failure memo ── +// +// The memo above exists for one thing (#10221): a `sys_setting` query that +// actively FAILS must not re-run — and re-log the driver's line — on every +// request. Its write condition was wider than that. `failed` was set by SIX +// legs and only ONE of them is the backend fault the cache's own docblock +// describes: +// +// settings.getMany(...) threw — the grouped read (settings leg) +// settings.get(...) threw × 3 — the older per-key arm (settings) +// the settings block threw — "service unavailable" (settings) +// ql.find('sys_setting', ...) threw — THE backend fault +// +// The five settings legs are reachable INSIDE the settings engine's bind +// window: `SettingsService.getMany` refuses all-or-nothing for a +// `localization` namespace whose manifest is not (yet) registered. So a +// caller that deliberately re-reads AFTER the bind — the #11580 repair does +// exactly that, re-resolving at `kernel:bootstrapped` — could be answered +// from the memo taken inside the window, within the 30s TTL, with nothing in +// the output saying the correction did not happen. +// +// And directly against this cache's own docblock ("a successful read is NEVER +// cached"): a settings refusal standing alongside a perfectly SUCCESSFUL +// direct read memoized that successful value for 30s — the exact staleness +// `analytics-timezone.dogfood.test.ts` (#1982/#2018) exists to forbid. +// +// So the memo is written only for the backend-fault leg now. #10221's +// protection is untouched, and BOTH directions are pinned below: a genuine +// backend fault still memoizes — including with a settings refusal standing +// in front of it — while a settings refusal alone no longer does. +describe('resolveLocalizationContext — only a backend fault populates the memo (#11877)', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + afterEach(() => { + vi.useRealTimers(); + }); + + /** What the workspace has PERSISTED — answerable only once the engine binds. */ + const CONFIGURED = { timezone: 'Asia/Shanghai', locale: 'zh-CN', currency: 'CNY' }; + + // The all-or-nothing refusal `SettingsService.getMany` gives for a namespace + // whose manifest is not (yet) registered. It throws out of an in-memory + // registry check — before any query, before any log line — which is why + // memoizing THIS leg never suppressed a query or a log line to begin with. + function makeBindWindowSettings() { + const state = { bound: false, getManyCalls: 0 }; + return { + state, + get: async () => { + throw new Error('per-key get must not be called on the batched arm'); + }, + getMany: async () => { + state.getManyCalls += 1; + if (!state.bound) throw new Error("unknown settings namespace 'localization'"); + return { + timezone: { value: CONFIGURED.timezone }, + locale: { value: CONFIGURED.locale }, + currency: { value: CONFIGURED.currency }, + }; + }, + }; + } + + // ── the reproduction this card was filed without ──────────────────────── + // + // Pre-bind read inside the window (settings refuses, `sys_setting` answers + // an ordinary empty result) → deliberate post-bind re-read 1s later, well + // inside the 30s TTL. The clock is FAKE and advanced explicitly; nothing + // here sleeps on the wall clock. + it('a post-bind re-read inside the TTL is answered by the now-bound service, not by the pre-bind memo', async () => { + const settings = makeBindWindowSettings(); + const ql = makeCountingQl({ sys_setting: [] }); + + const preBind = await resolveLocalizationContext({ ql, settings, tenantId: 'o1' }); + expect(preBind).toEqual({ timezone: 'UTC', locale: 'en-US', currency: undefined }); + expect(settings.state.getManyCalls).toBe(1); + + settings.state.bound = true; // the settings engine binds + await vi.advanceTimersByTimeAsync(1_000); // still deep inside the 30s window + + const postBind = await resolveLocalizationContext({ ql, settings, tenantId: 'o1' }); + // The re-read must REACH the service (a memo hit would never call it) and + // must carry the configured values, which exist only behind the bind. + expect(settings.state.getManyCalls).toBe(2); + expect(postBind).toEqual({ timezone: 'Asia/Shanghai', locale: 'zh-CN', currency: 'CNY' }); + }); + + // The same refusal, but the direct read SUCCEEDS with rows. The memoized + // value here was a correct, successful answer — frozen for 30s by a leg that + // has nothing to do with the backend. + it('a settings refusal never freezes a SUCCESSFUL direct read: a row change is visible on the very next call', async () => { + const settings = makeBindWindowSettings(); // stays unbound → refuses every call + const rows = [{ namespace: 'localization', key: 'timezone', scope: 'tenant', value: 'UTC' }]; + const ql = makeCountingQl({ sys_setting: rows }); + + const first = await resolveLocalizationContext({ ql, settings, tenantId: 'o1' }); + expect(first.timezone).toBe('UTC'); + + rows[0].value = 'America/Los_Angeles'; // a settings write lands, no TTL advance + const second = await resolveLocalizationContext({ ql, settings, tenantId: 'o1' }); + expect(second.timezone).toBe('America/Los_Angeles'); + expect(ql.counts.sys_setting).toBe(2); + }); + + // The older per-key arm (a service with no `getMany`): three `get` legs, + // same rule. + it('the per-key get legs do not populate the memo either', async () => { + const state = { bound: false, gets: 0 }; + const settings = { + get: async (_ns: string, key: string) => { + state.gets += 1; + if (!state.bound) throw new Error("unknown settings namespace 'localization'"); + return { value: (CONFIGURED as Record)[key] }; + }, + }; + const ql = makeCountingQl({ sys_setting: [] }); + + expect(await resolveLocalizationContext({ ql, settings, tenantId: 'o1' })).toEqual({ + timezone: 'UTC', + locale: 'en-US', + currency: undefined, + }); + expect(state.gets).toBe(3); + + state.bound = true; + await vi.advanceTimersByTimeAsync(1_000); + expect(await resolveLocalizationContext({ ql, settings, tenantId: 'o1' })).toEqual({ + timezone: 'Asia/Shanghai', + locale: 'zh-CN', + currency: 'CNY', + }); + expect(state.gets).toBe(6); + }); + + // The whole-block leg: a `get` that throws SYNCHRONOUSLY never attaches its + // `.catch`, so the throw escapes `Promise.all` into the outer + // "settings service unavailable → direct read" handler. Same rule. + it('the outer "settings service unavailable" leg does not populate the memo either', async () => { + const state = { bound: false, gets: 0 }; + const settings = { + // Deliberately NOT async: this throws before a promise exists. + get: (_ns: string, key: string) => { + state.gets += 1; + if (!state.bound) throw new Error('settings service unavailable'); + return Promise.resolve({ value: (CONFIGURED as Record)[key] }); + }, + }; + const ql = makeCountingQl({ sys_setting: [] }); + + expect(await resolveLocalizationContext({ ql, settings, tenantId: 'o1' })).toEqual({ + timezone: 'UTC', + locale: 'en-US', + currency: undefined, + }); + + state.bound = true; + await vi.advanceTimersByTimeAsync(1_000); + expect((await resolveLocalizationContext({ ql, settings, tenantId: 'o1' })).timezone).toBe('Asia/Shanghai'); + }); + + // ── the half that must be PRESERVED (#10221) ──────────────────────────── + // + // Narrowing the write condition must not narrow it to nothing. A settings + // refusal standing in FRONT of a genuinely failing `sys_setting` read is the + // real #10221 environment (fresh deployment: no manifest registered yet AND + // no table yet) — the failing query must still be memoized there. + it('still memoizes when a settings refusal stands in front of a genuine backend fault', async () => { + const settings = makeBindWindowSettings(); // unbound → refuses + const ql = makeMissingTableQl(); // and the direct read throws + + await resolveLocalizationContext({ ql, settings, tenantId: 'o1' }); + expect(ql.counts.sys_setting).toBe(1); + + await vi.advanceTimersByTimeAsync(1_000); + await resolveLocalizationContext({ ql, settings, tenantId: 'o1' }); + // Memo hit: the failing query — and the driver's log line for it — did not + // repeat. The settings refusal is re-attempted (it is free), but that is + // not what #10221 was protecting. + expect(ql.counts.sys_setting).toBe(1); + + await vi.advanceTimersByTimeAsync(30_001); + await resolveLocalizationContext({ ql, settings, tenantId: 'o1' }); + expect(ql.counts.sys_setting).toBe(2); // and it still self-heals on expiry + }); +}); + describe('grant validity windows (ADR-0091 D1/D2)', () => { const NOW = Date.parse('2026-07-10T12:00:00Z'); const PAST = '2026-07-01T00:00:00Z'; diff --git a/packages/core/src/security/resolve-authz-context.ts b/packages/core/src/security/resolve-authz-context.ts index 2b6e227b81..475e6a6166 100644 --- a/packages/core/src/security/resolve-authz-context.ts +++ b/packages/core/src/security/resolve-authz-context.ts @@ -771,6 +771,25 @@ type LocalizationResult = { timezone: string; locale: string; currency?: string * where the read is actively failing — while never caching a value a caller * could observe going stale. * + * [#11877] "the underlying read itself THREW" means the DIRECT `sys_setting` + * read, and only it. The write condition used to be a single `failed` flag + * that five SETTINGS-SERVICE legs also set (a thrown `getMany`, each of the + * three older per-key `get`s, and the whole-block "service unavailable" + * handler). Those legs are reachable inside the settings engine's BIND + * WINDOW — `SettingsService.getMany` refuses all-or-nothing for a namespace + * whose manifest is not yet registered — so a caller that deliberately + * re-reads AFTER the bind (the #11580 repair re-resolves at + * `kernel:bootstrapped`) was answered from the in-window memo for up to 30s, + * silently keeping the very value the re-read existed to replace. And a + * settings refusal standing alongside a SUCCESSFUL direct read memoized that + * successful value — exactly the staleness the paragraph above forbids. + * + * Narrowing to the backend leg costs nothing #10221 bought: those refusals + * throw out of an in-memory registry check BEFORE any query and before any + * log line, so memoizing them suppressed neither — while #10221's own + * environment (table not migrated yet) still memoizes, because the direct + * read throws there whether or not a settings refusal stands in front of it. + * * Keyed on the `ql` instance (one entry-set per environment engine, so two * environments/tenants sharing a process never see each other's cached * outcome) and then `tenantId|userId` beneath it, matching the audit writer's @@ -786,10 +805,12 @@ const localizationFailureCache = new WeakMap Date.now()) return hit.value; } - const { value, failed } = await resolveLocalizationContextUncached(input); - if (failed && ql && typeof ql === 'object') { + const { value, backendFailed } = await resolveLocalizationContextUncached(input); + if (backendFailed && ql && typeof ql === 'object') { const bucket = localizationFailureCache.get(ql) ?? new Map(); bucket.set(cacheKey, { value, expiresAt: Date.now() + LOCALIZATION_FAILURE_CACHE_TTL_MS }); localizationFailureCache.set(ql, bucket); @@ -814,9 +835,11 @@ export async function resolveLocalizationContext(input: ResolveLocalizationInput async function resolveLocalizationContextUncached( input: ResolveLocalizationInput, -): Promise<{ value: LocalizationResult; failed: boolean }> { +): Promise<{ value: LocalizationResult; backendFailed: boolean }> { const { ql, settings, tenantId, userId } = input; - let failed = false; + // ONLY the direct `sys_setting` read below sets this. The settings-service + // legs deliberately do not — see the cache doc above (#11877). + let backendFailed = false; try { if (settings && typeof settings.get === 'function') { const sctx = { tenantId, userId } as any; @@ -826,9 +849,10 @@ async function resolveLocalizationContextUncached( // answers by the service's own equivalence contract. Feature-detected: // an older service without `getMany` keeps the three parallel `get`s // (still 1 leg — this is a query-count fix, per the card's calibration). - // A thrown `getMany` lands in the same place a thrown `get` did — - // `failed = true` and the direct `$in` fallback below, which reads the - // exact same three keys. + // A thrown `getMany` lands in the same place a thrown `get` did — the + // direct `$in` fallback below, which reads the exact same three keys. + // Neither populates the failure memo: a settings refusal is not the + // backend fault that memo is for (#11877; see the cache doc above). // // [#11222 item 4] ONE non-equivalence, inherent to batching and recorded // here because it is this CALLER's degradation, not the service's: @@ -850,34 +874,28 @@ async function resolveLocalizationContextUncached( localeRes = many.locale; currencyRes = many.currency; } catch { - failed = true; + // Settings refusal → fall through to the direct `$in` read below. + // Not a backend fault, so it does not populate the memo (#11877). } } else { + // Same rule as the batched arm above: a refused key falls through to + // the direct `$in` read and does not populate the memo (#11877). [tzRes, localeRes, currencyRes] = await Promise.all([ - settings.get('localization', 'timezone', sctx).catch(() => { - failed = true; - return undefined; - }), - settings.get('localization', 'locale', sctx).catch(() => { - failed = true; - return undefined; - }), - settings.get('localization', 'currency', sctx).catch(() => { - failed = true; - return undefined; - }), + settings.get('localization', 'timezone', sctx).catch(() => undefined), + settings.get('localization', 'locale', sctx).catch(() => undefined), + settings.get('localization', 'currency', sctx).catch(() => undefined), ]); } const tz = coerceTimeZone(tzRes?.value); const locale = coerceLocale(localeRes?.value); const currency = coerceCurrency(currencyRes?.value); if (tz || locale || currency) { - return { value: { timezone: tz ?? 'UTC', locale: locale ?? 'en-US', currency }, failed: false }; + return { value: { timezone: tz ?? 'UTC', locale: locale ?? 'en-US', currency }, backendFailed: false }; } } } catch { - // settings service unavailable → direct read - failed = true; + // Settings service unavailable → direct read. Still not a backend fault, + // so it does not populate the memo either (#11877). } // One read for all three keys instead of a query per key (`$in` on `key`). // Inlined (rather than the shared `tryFind`) so a genuine backend fault — @@ -898,7 +916,8 @@ async function resolveLocalizationContextUncached( if (result && (result as any).value) result = (result as any).value; rows = Array.isArray(result) ? result : []; } catch { - failed = true; + // THE backend fault the failure memo exists for (#10221). + backendFailed = true; } } const valueOf = (k: string) => rows.find((r) => r.key === k)?.value; @@ -908,6 +927,6 @@ async function resolveLocalizationContextUncached( locale: coerceLocale(valueOf('locale')) ?? 'en-US', currency: coerceCurrency(valueOf('currency')), }, - failed, + backendFailed, }; }