diff --git a/apps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.test.ts b/apps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.test.ts index ff8c178ee..e6a7898b8 100644 --- a/apps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.test.ts +++ b/apps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.test.ts @@ -1,5 +1,11 @@ import { describe, expect, it } from "vitest"; -import { computeMintShard, type MintShardDeps } from "./runOpsMintShard.server"; +import { + computeMintShard, + resolveMintShardWith, + type MintShardCache, + type MintShardDeps, + type ResolveMintShardDeps, +} from "./runOpsMintShard.server"; import { type MintShardSetResolution } from "./mintShardGrace"; const GRACE_MS = 90_000; @@ -297,3 +303,92 @@ describe("computeMintShard — the ceiling bounds the stored list", () => { expect(shard).toBe("new"); }); }); + +describe("resolveMintShardWith — cache, ceiling short-circuit and fail-safe", () => { + function wrapperDeps( + overrides: Partial = {} + ): ResolveMintShardDeps & { reads: number } { + const state = { + ceiling: ["a", "b"], + readFlags: async () => ({ runOpsMintShardSet: "a,b" }), + cache: { current: undefined as MintShardCache }, + nowMs: T, + ttlMs: 30_000, + graceMs: GRACE_MS, + orgFeatureFlags: undefined as unknown, + reads: 0, + ...overrides, + }; + const wrapped = state.readFlags; + state.readFlags = async () => { + state.reads++; + return wrapped(); + }; + return state; + } + + it("never reads the list when the deployment configures no ceiling", async () => { + const deps = wrapperDeps({ ceiling: [] }); + expect(await resolveMintShardWith({ id: "env_1" }, deps)).toBe("new"); + expect(deps.reads).toBe(0); + }); + + it("reads once, then serves the cache until the TTL expires", async () => { + const deps = wrapperDeps(); + await resolveMintShardWith({ id: "env_1" }, deps); + await resolveMintShardWith({ id: "env_2" }, deps); + await resolveMintShardWith({ id: "env_3" }, deps); + expect(deps.reads).toBe(1); + }); + + it("reads again once the TTL expires", async () => { + const deps = wrapperDeps(); + await resolveMintShardWith({ id: "env_1" }, deps); + deps.nowMs = T + 30_000; + await resolveMintShardWith({ id: "env_1" }, deps); + expect(deps.reads).toBe(2); + }); + + it("falls back to gen-1 when the read throws, and does not poison the cache", async () => { + // A blip must not move every environment's placement, so it returns gen-1 rather than guess. + let fail = true; + const deps = wrapperDeps({ + readFlags: async () => { + if (fail) throw new Error("db down"); + return { runOpsMintShardSet: "a,b" }; + }, + }); + const failures: unknown[] = []; + deps.onReadFailed = (error) => failures.push(error); + + expect(await resolveMintShardWith({ id: "env_1" }, deps)).toBe("new"); + expect(failures).toHaveLength(1); + + fail = false; + expect(["a", "b"]).toContain(await resolveMintShardWith({ id: "env_1" }, deps)); + }); + + it("returns gen-1 when the stored list names nothing inside the ceiling", async () => { + const deps = wrapperDeps({ + ceiling: ["a"], + readFlags: async () => ({ runOpsMintShardSet: "z" }), + }); + expect(await resolveMintShardWith({ id: "env_1" }, deps)).toBe("new"); + }); + + it("agrees with the pure core for the same inputs", async () => { + const deps = wrapperDeps(); + const viaWrapper = await resolveMintShardWith({ id: "env_1" }, deps); + const viaCore = computeMintShard( + { id: "env_1" }, + { + resolution: { set: ["a", "b"] }, + ceiling: ["a", "b"], + nowMs: T, + graceMs: GRACE_MS, + orgFeatureFlags: undefined, + } + ); + expect(viaWrapper).toBe(viaCore); + }); +}); diff --git a/apps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.ts b/apps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.ts index 8080acce0..540f10b56 100644 --- a/apps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.ts +++ b/apps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.ts @@ -123,22 +123,70 @@ export function computeMintShard(environment: { id: string }, deps: MintShardDep // deploy-time value, so re-parsing per mint would burn CPU on the hottest path in the system. const ceiling: string[] = parseShardCsv(env.RUN_OPS_MINT_SHARDS); -// The live list is org-independent, so one process-wide entry serves every mint. One query per -// process per TTL, folded into a single round-trip over the three keys. The TTL bounds how long -// two processes can disagree, which is what the grace window is sized against. const SET_KEYS = [ FEATURE_FLAG.runOpsMintShardSet, FEATURE_FLAG.runOpsMintShardSetPrev, FEATURE_FLAG.runOpsMintShardSetFlippedAt, ]; -let cachedResolution: { value: MintShardSetResolution; expiresAt: number } | undefined; +export type MintShardCache = { value: MintShardSetResolution; expiresAt: number } | undefined; -async function readLiveResolution(): Promise { - if (cachedResolution && cachedResolution.expiresAt > Date.now()) { - return cachedResolution.value; +export type ResolveMintShardDeps = { + ceiling: string[]; + // Reads the three list rows. Injected so the cache and the fail-safe are testable without a + // database, the same way computeRunIdMintKind takes its flag reader. + readFlags: () => Promise>; + cache: { current: MintShardCache }; + nowMs: number; + ttlMs: number; + graceMs: number; + orgFeatureFlags: unknown; + onPinRejected?: (info: { environmentId: string; pin: string; activeSet: string[] }) => void; + onReadFailed?: (error: unknown) => void; +}; + +// The live list is org-independent, so one process-wide entry serves every mint. One query per +// process per TTL, over one round-trip. The TTL bounds how long two processes can disagree, +// which is what the grace window is sized against. +// +// A failed read falls back to gen-1 rather than guessing a list. Guessing would move every +// environment's placement for the length of one blip. +export async function resolveMintShardWith( + environment: { id: string; orgFeatureFlags?: unknown }, + deps: ResolveMintShardDeps +): Promise { + // No ceiling means no gen-2 minting, so skip the read entirely. + if (deps.ceiling.length === 0) { + return "new"; } + let resolution: MintShardSetResolution; + const cached = deps.cache.current; + if (cached && cached.expiresAt > deps.nowMs) { + resolution = cached.value; + } else { + try { + resolution = readMintShardSetResolution(await deps.readFlags()); + } catch (error) { + deps.onReadFailed?.(error); + return "new"; + } + deps.cache.current = { value: resolution, expiresAt: deps.nowMs + deps.ttlMs }; + } + + return computeMintShard(environment, { + resolution, + ceiling: deps.ceiling, + nowMs: deps.nowMs, + graceMs: deps.graceMs, + orgFeatureFlags: deps.orgFeatureFlags, + onPinRejected: deps.onPinRejected, + }); +} + +const liveCache: { current: MintShardCache } = { current: undefined }; + +async function readSetFlags(): Promise> { const rows = await $replica.featureFlag.findMany({ where: { key: { in: SET_KEYS } }, select: { key: true, value: true }, @@ -147,10 +195,7 @@ async function readLiveResolution(): Promise { for (const row of rows) { flags[row.key] = row.value; } - - const value = readMintShardSetResolution(flags); - cachedResolution = { value, expiresAt: Date.now() + env.RUN_OPS_MINT_FLAG_CACHE_TTL_MS }; - return value; + return flags; } // Once per environment per process: a stale pin sits on the root-trigger path and would @@ -178,26 +223,16 @@ export async function resolveMintShard(environment: { // Pass environment.organization.featureFlags from the trigger call site. orgFeatureFlags?: unknown; }): Promise { - // No ceiling means no gen-2 minting, so skip the read entirely. - if (ceiling.length === 0) { - return "new"; - } - - let resolution: MintShardSetResolution; - try { - resolution = await readLiveResolution(); - } catch (error) { - // Fail safe to gen-1, mirroring the mint-kind gate's fail-safe to cuid. - logger.error("[runOpsMintShard] shard-set read failed; minting gen-1 (fail-safe)", { error }); - return "new"; - } - - return computeMintShard(environment, { - resolution, + return resolveMintShardWith(environment, { ceiling, + readFlags: readSetFlags, + cache: liveCache, nowMs: Date.now(), + ttlMs: env.RUN_OPS_MINT_FLAG_CACHE_TTL_MS, graceMs: env.RUN_OPS_MINT_FLIP_GRACE_MS, orgFeatureFlags: environment.orgFeatureFlags, onPinRejected: reportPinRejected, + onReadFailed: (error) => + logger.error("[runOpsMintShard] shard-set read failed; minting gen-1 (fail-safe)", { error }), }); } diff --git a/apps/webapp/test/runOpsMintShardFlags.test.ts b/apps/webapp/test/runOpsMintShardFlags.test.ts new file mode 100644 index 000000000..81ce7ed7f --- /dev/null +++ b/apps/webapp/test/runOpsMintShardFlags.test.ts @@ -0,0 +1,100 @@ +// The mint-shard flags carry two safety claims that only the catalog can enforce: a bad value is +// rejected at WRITE (so no unroutable key and no silently-unpinned environment can ever be +// stored), and each key is locked at the scope its resolver does not read. Pure, no containers. +import { describe, expect, it } from "vitest"; +import { + FEATURE_FLAG, + FeatureFlagCatalog, + GLOBAL_LOCKED_FLAGS, + ORG_LOCKED_FLAGS, + validateFeatureFlagValue, +} from "~/v3/featureFlags"; + +describe("runOpsMintShard — the per-org pin", () => { + const key = FEATURE_FLAG.runOpsMintShard; + + it("accepts every legal shard key", () => { + for (const c of "abcdefghijklmnopqrstuvwxyz0123456789") { + expect(validateFeatureFlagValue(key, c).success).toBe(true); + } + }); + + it('accepts "new", which holds an org on gen-1', () => { + expect(validateFeatureFlagValue(key, "new").success).toBe(true); + }); + + it("rejects a value that could never be stamped into an id", () => { + for (const bad of ["A", "ab", "", "-", "legacy", " a", "a,b"]) { + expect(validateFeatureFlagValue(key, bad).success).toBe(false); + } + }); +}); + +describe("runOpsMintShardEnvPins — the per-environment pins", () => { + const key = FEATURE_FLAG.runOpsMintShardEnvPins; + + it("accepts a map of environment id to shard key", () => { + expect( + validateFeatureFlagValue(key, JSON.stringify({ env_1: "a", env_2: "new" })).success + ).toBe(true); + expect(validateFeatureFlagValue(key, "{}").success).toBe(true); + }); + + it("rejects a blob that is not JSON, so a typo cannot silently un-pin every environment", () => { + for (const bad of ["{not json", "", "null", "[]", '"a"', "42"]) { + expect(validateFeatureFlagValue(key, bad).success).toBe(false); + } + }); + + it("rejects a map whose value is not a legal pin", () => { + for (const bad of [{ env_1: "AB" }, { env_1: "legacy" }, { env_1: 1 }, { env_1: "" }]) { + expect(validateFeatureFlagValue(key, JSON.stringify(bad)).success).toBe(false); + } + }); +}); + +describe("runOpsMintShardSet — the active list", () => { + const key = FEATURE_FLAG.runOpsMintShardSet; + + it("accepts an empty list and a CSV of legal keys", () => { + expect(validateFeatureFlagValue(key, "").success).toBe(true); + expect(validateFeatureFlagValue(key, "a").success).toBe(true); + expect(validateFeatureFlagValue(key, "a,b, c").success).toBe(true); + }); + + it("rejects a CSV holding a key that cannot be routed", () => { + for (const bad of ["A", "ab", "a,B", "a,legacy", "a,new", "a;b"]) { + expect(validateFeatureFlagValue(key, bad).success).toBe(false); + } + }); +}); + +describe("scope locks match what each resolver actually reads", () => { + it("locks the pins globally, because the resolver reads them from the org blob only", () => { + expect(GLOBAL_LOCKED_FLAGS).toContain(FEATURE_FLAG.runOpsMintShard); + expect(GLOBAL_LOCKED_FLAGS).toContain(FEATURE_FLAG.runOpsMintShardEnvPins); + }); + + it("locks the list per-org, because it is deployment-wide", () => { + expect(ORG_LOCKED_FLAGS).toContain(FEATURE_FLAG.runOpsMintShardSet); + expect(ORG_LOCKED_FLAGS).toContain(FEATURE_FLAG.runOpsMintShardSetPrev); + expect(ORG_LOCKED_FLAGS).toContain(FEATURE_FLAG.runOpsMintShardSetFlippedAt); + }); + + it("keeps the pins settable per-org, which is the canary lever", () => { + expect(ORG_LOCKED_FLAGS).not.toContain(FEATURE_FLAG.runOpsMintShard); + expect(ORG_LOCKED_FLAGS).not.toContain(FEATURE_FLAG.runOpsMintShardEnvPins); + }); + + it("registers every new key in the catalog, so the admin pages render it", () => { + for (const key of [ + FEATURE_FLAG.runOpsMintShard, + FEATURE_FLAG.runOpsMintShardEnvPins, + FEATURE_FLAG.runOpsMintShardSet, + FEATURE_FLAG.runOpsMintShardSetPrev, + FEATURE_FLAG.runOpsMintShardSetFlippedAt, + ]) { + expect(FeatureFlagCatalog).toHaveProperty(key); + } + }); +});