From 5ce9ca42f423366e3c5bedd8fe2343575d8148eb Mon Sep 17 00:00:00 2001 From: os-warren Date: Tue, 25 Aug 2026 06:13:31 +0000 Subject: [PATCH] fix(core): only a backend fault populates the localization failure memo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `resolveLocalizationContext` memoized an outcome for 30s whenever the read "failed", but six legs set that flag and only one of them is the backend fault the cache's own docblock describes (the direct `ql.find` throw). The other five are the settings service refusing — a thrown `getMany`, the three older per-key `get`s, and the whole-block "service unavailable" handler. Those five are reachable inside the settings engine's bind window, so a caller that deliberately re-reads after the bind was answered from the in-window memo for up to 30s, and a settings refusal standing alongside a SUCCESSFUL direct read froze that successful value — the staleness the docblock forbids outright. 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 still memoizes, because the direct read throws there whether or not a settings refusal stands in front of it. Both directions are pinned. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01W6HFzyH98W1YaQXhJUJt6o --- ...alization-failure-memo-backend-leg-only.md | 37 +++ .../security/resolve-authz-context.test.ts | 216 ++++++++++++++++-- .../src/security/resolve-authz-context.ts | 77 ++++--- 3 files changed, 288 insertions(+), 42 deletions(-) create mode 100644 .changeset/localization-failure-memo-backend-leg-only.md 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, }; }