Skip to content
Closed
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
4 changes: 4 additions & 0 deletions apps/webapp/app/env.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1753,6 +1753,10 @@ const EnvironmentSchema = z
RUN_OPS_MINT_ENABLED: BoolEnv.default(false),
RUN_OPS_MINT_FLAG_CACHE_TTL_MS: z.coerce.number().int().default(30_000),
RUN_OPS_MINT_FLAG_CACHE_MAX_ENTRIES: z.coerce.number().int().default(10_000),
// Deterministic wall-clock cutover after a runOpsMintKind flip. Must exceed the sum
// of RUN_OPS_MINT_FLAG_CACHE_TTL_MS and the control-plane cache TTL so every process
// (stale or fresh) resolves to the same kind for the whole window. See mintFlipGrace.ts.
RUN_OPS_MINT_FLIP_GRACE_MS: z.coerce.number().int().default(90_000),

// Session replication (Postgres → ClickHouse sessions_v1). Shares Redis
// with the runs replicator for leader locking but has its own slot and
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import type { ActionFunctionArgs, LoaderFunctionArgs } from "@remix-run/server-runtime";
import { json } from "@remix-run/server-runtime";
import type { Prisma } from "@trigger.dev/database";
import { z } from "zod";
import { env } from "~/env.server";
import { prisma } from "~/db.server";
import { requireAdminApiRequest } from "~/services/personalAccessToken.server";
import { controlPlaneResolver } from "~/v3/runOpsMigration/controlPlaneResolver.server";
import { stampMintKindFlip } from "~/v3/runOpsMigration/mintFlipGrace";
import { validatePartialFeatureFlags } from "~/v3/featureFlags";

const ParamsSchema = z.object({
Expand Down Expand Up @@ -82,18 +85,23 @@ export async function action({ request, params }: ActionFunctionArgs) {
? validatePartialFeatureFlags(organization.featureFlags as Record<string, unknown>)
: { success: false as const };

const mergedFlags = {
...(existingFlags.success ? existingFlags.data : {}),
...validationResult.data,
};
const mergedFlags = stampMintKindFlip(
existingFlags.success ? existingFlags.data : {},
{
...(existingFlags.success ? existingFlags.data : {}),
...validationResult.data,
},
Date.now(),

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: Use control plane DB time

env.RUN_OPS_MINT_FLIP_GRACE_MS
);

// Update the organization's feature flags
const updatedOrganization = await prisma.organization.update({
where: {
id: organizationId,
},
data: {
featureFlags: mergedFlags,
featureFlags: mergedFlags as Prisma.InputJsonValue,
},
select: {
id: true,
Expand Down

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Grace stamp is lost when all flags are cleared via the v2 route

When the v2 admin route receives null or {} as the body, it sets featureFlags = Prisma.JsonNull (line 114) and skips stampMintKindFlip entirely. This means clearing all flags during an active grace window will immediately lose the grace stamp (runOpsMintKindPrev, runOpsMintKindFlippedAt), potentially causing cross-process divergence if a stale process still has the old kind cached. This is likely acceptable since clearing all flags is a deliberate reset, but operators should be aware that clearing flags mid-grace is not safe for the mint-kind convergence guarantee.

(Refers to lines 110-114)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ import type { ActionFunctionArgs, LoaderFunctionArgs } from "@remix-run/server-r
import { json } from "@remix-run/server-runtime";
import { Prisma } from "@trigger.dev/database";
import { z } from "zod";
import { env } from "~/env.server";
import { prisma } from "~/db.server";
import { requireUser } from "~/services/session.server";
import { controlPlaneResolver } from "~/v3/runOpsMigration/controlPlaneResolver.server";
import { stampMintKindFlip } from "~/v3/runOpsMigration/mintFlipGrace";
import { flags as getGlobalFlags } from "~/v3/featureFlags.server";
import {
FEATURE_FLAG,
Expand Down Expand Up @@ -119,6 +121,17 @@ export async function action({ request, params }: ActionFunctionArgs) {
);
}
featureFlags = validationResult.data;

const existingOrg = await prisma.organization.findFirst({
where: { id: organizationId },
select: { featureFlags: true },
});
featureFlags = stampMintKindFlip(
existingOrg?.featureFlags as Record<string, unknown> | null | undefined,
featureFlags,
Date.now(),

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: use control plane db time

env.RUN_OPS_MINT_FLIP_GRACE_MS
);
}

try {
Expand Down
9 changes: 9 additions & 0 deletions apps/webapp/app/v3/featureFlags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ export const FEATURE_FLAG = {
computeMigrationRequireTemplate: "computeMigrationRequireTemplate",
devBranchesEnabled: "devBranchesEnabled",
runOpsMintKind: "runOpsMintKind",
// Grace-linger stamp carried alongside runOpsMintKind on flip. See mintFlipGrace.ts.
runOpsMintKindPrev: "runOpsMintKindPrev",
runOpsMintKindFlippedAt: "runOpsMintKindFlippedAt",
} as const;

export const FeatureFlagCatalog = {
Expand Down Expand Up @@ -54,6 +57,10 @@ export const FeatureFlagCatalog = {
// Per-org run-ops-id mint cutover. Defaults to "cuid"; only honored when
// RUN_OPS_MINT_ENABLED is on AND isSplitEnabled() is true.
[FEATURE_FLAG.runOpsMintKind]: z.enum(["cuid", "runOpsId"]),
// Grace-linger stamp: the previously-effective kind and the flip timestamp, written
// by stampMintKindFlip on a genuine flip. Display-only (see ORG_LOCKED_FLAGS).
[FEATURE_FLAG.runOpsMintKindPrev]: z.enum(["cuid", "runOpsId"]),
[FEATURE_FLAG.runOpsMintKindFlippedAt]: z.string().datetime(),
};

export type FeatureFlagKey = keyof typeof FeatureFlagCatalog;
Expand All @@ -70,6 +77,8 @@ export const GLOBAL_LOCKED_FLAGS: FeatureFlagKey[] = [
export const ORG_LOCKED_FLAGS: FeatureFlagKey[] = [
FEATURE_FLAG.defaultWorkerInstanceGroupId,
FEATURE_FLAG.taskEventRepository,
FEATURE_FLAG.runOpsMintKindPrev,
FEATURE_FLAG.runOpsMintKindFlippedAt,
];

// Create a Zod schema from the existing catalog
Expand Down
159 changes: 159 additions & 0 deletions apps/webapp/app/v3/runOpsMigration/mintFlipGrace.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { effectiveMintKind, stampMintKindFlip, type MintFlagResolution } from "./mintFlipGrace";

// GRACE-LINGER: during [flippedAt, flippedAt + GRACE) every process — stale or fresh —
// must resolve to the SAME (old) kind; at/after the cutover every process resolves to
// the SAME (new) kind. This collapses the cross-process divergence window.
const GRACE_MS = 90_000;
const T = 1_000_000;

describe("effectiveMintKind", () => {
beforeEach(() => vi.useFakeTimers());
afterEach(() => vi.useRealTimers());

it("returns r.kind directly when prev is missing", () => {
const r: MintFlagResolution = { kind: "cuid" };
expect(effectiveMintKind(r, T, GRACE_MS)).toBe("cuid");
});

it("returns r.kind directly when flippedAtMs is missing", () => {
const r: MintFlagResolution = { kind: "runOpsId", prev: "cuid" };
expect(effectiveMintKind(r, T, GRACE_MS)).toBe("runOpsId");
});

it("CORE: stale and fresh resolutions agree at every instant during grace, then both flip to the new kind at cutover", () => {
const stale: MintFlagResolution = { kind: "cuid" };
const fresh: MintFlagResolution = { kind: "runOpsId", prev: "cuid", flippedAtMs: T };

for (const now of [T, T + 1, T + 1_000, T + 45_000, T + GRACE_MS - 1]) {
const staleResolved = effectiveMintKind(stale, now, GRACE_MS);
const freshResolved = effectiveMintKind(fresh, now, GRACE_MS);
expect(staleResolved).toBe("cuid");
expect(freshResolved).toBe("cuid");
}

for (const now of [T + GRACE_MS, T + GRACE_MS + 1, T + GRACE_MS + 60_000]) {
expect(effectiveMintKind(fresh, now, GRACE_MS)).toBe("runOpsId");
}
});

it("boundary: exactly at flippedAt + GRACE resolves to the NEW kind", () => {
const fresh: MintFlagResolution = { kind: "runOpsId", prev: "cuid", flippedAtMs: T };
expect(effectiveMintKind(fresh, T + GRACE_MS - 1, GRACE_MS)).toBe("cuid");
expect(effectiveMintKind(fresh, T + GRACE_MS, GRACE_MS)).toBe("runOpsId");
});
});

describe("stampMintKindFlip", () => {
beforeEach(() => vi.useFakeTimers());
afterEach(() => vi.useRealTimers());

it("a genuine flip (cuid -> runOpsId) stamps prev and flippedAt", () => {
const existing = { runOpsMintKind: "cuid" };
const outgoing = { runOpsMintKind: "runOpsId" };
const result = stampMintKindFlip(existing, outgoing, T, GRACE_MS);

expect(result.runOpsMintKind).toBe("runOpsId");
expect(result.runOpsMintKindPrev).toBe("cuid");
expect(result.runOpsMintKindFlippedAt).toBe(new Date(T).toISOString());
});

it("defaults the existing effective kind to cuid when existing flags are null", () => {
const outgoing = { runOpsMintKind: "runOpsId" };
const result = stampMintKindFlip(null, outgoing, T, GRACE_MS);

expect(result.runOpsMintKind).toBe("runOpsId");
expect(result.runOpsMintKindPrev).toBe("cuid");
expect(result.runOpsMintKindFlippedAt).toBe(new Date(T).toISOString());
});

it("resubmitting the same target kind mid-grace carries the stamp forward untouched (does not reset the cutover clock)", () => {
const existing = {
runOpsMintKind: "runOpsId",
runOpsMintKindPrev: "cuid",
runOpsMintKindFlippedAt: new Date(T).toISOString(),
};
const now = T + 10_000;
const outgoing = { runOpsMintKind: "runOpsId", someOtherFlag: true };
const result = stampMintKindFlip(existing, outgoing, now, GRACE_MS);

expect(result.runOpsMintKind).toBe("runOpsId");
expect(result.runOpsMintKindPrev).toBe("cuid");
// Unrelated re-save: the cutover time must NOT slide forward.
expect(result.runOpsMintKindFlippedAt).toBe(new Date(T).toISOString());
expect(result.someOtherFlag).toBe(true);
});

it("an unchanged save after grace has elapsed carries the settled stamp forward and preserves unrelated flags", () => {
const existing = {
runOpsMintKind: "runOpsId",
runOpsMintKindPrev: "cuid",
runOpsMintKindFlippedAt: new Date(T).toISOString(),
};
const outgoing = { runOpsMintKind: "runOpsId", someOtherFlag: true };
const result = stampMintKindFlip(existing, outgoing, T + GRACE_MS + 5_000, GRACE_MS);

expect(result.runOpsMintKind).toBe("runOpsId");
expect(result.someOtherFlag).toBe(true);
expect(result.runOpsMintKindPrev).toBe("cuid");
expect(result.runOpsMintKindFlippedAt).toBe(new Date(T).toISOString());
});

it("a flip-back requested after the original grace has elapsed stamps prev := the new settled (now-effective) kind, timestamped now", () => {
const existing = {
runOpsMintKind: "runOpsId",
runOpsMintKindPrev: "cuid",
runOpsMintKindFlippedAt: new Date(T).toISOString(),
};
const now = T + GRACE_MS + 1_000;
const outgoing = { runOpsMintKind: "cuid" };
const result = stampMintKindFlip(existing, outgoing, now, GRACE_MS);

expect(result.runOpsMintKind).toBe("cuid");
expect(result.runOpsMintKindPrev).toBe("runOpsId");
expect(result.runOpsMintKindFlippedAt).toBe(new Date(now).toISOString());
});

it("a flip-back mid-grace re-stamps prev to the still-effective old kind, so it keeps serving that kind (no divergence)", () => {
const existing = {
runOpsMintKind: "runOpsId",
runOpsMintKindPrev: "cuid",
runOpsMintKindFlippedAt: new Date(T).toISOString(),
};
const now = T + 20_000;
const outgoing = { runOpsMintKind: "cuid" };
const result = stampMintKindFlip(existing, outgoing, now, GRACE_MS);

expect(result.runOpsMintKind).toBe("cuid");
expect(result.runOpsMintKindPrev).toBe("cuid");
expect(result.runOpsMintKindFlippedAt).toBe(new Date(now).toISOString());
});

it("defaults outgoing kind to 'cuid' when runOpsMintKind is absent", () => {
const existing = { runOpsMintKind: "runOpsId" };
const outgoing: Record<string, unknown> = {};
const result = stampMintKindFlip(existing, outgoing, T, GRACE_MS);

expect(result.runOpsMintKind).toBe("cuid");
expect(result.runOpsMintKindPrev).toBe("runOpsId");
expect(result.runOpsMintKindFlippedAt).toBe(new Date(T).toISOString());
});

it("treats a malformed existing flippedAt as no stamp", () => {
const existing = {
runOpsMintKind: "runOpsId",
runOpsMintKindPrev: "cuid",
runOpsMintKindFlippedAt: "not-a-date",
};
const outgoing = { runOpsMintKind: "runOpsId" };
const result = stampMintKindFlip(existing, outgoing, T, GRACE_MS);

expect(result.runOpsMintKind).toBe("runOpsId");
// The malformed flippedAt is carried forward verbatim, but an unparseable timestamp is
// inert when resolved (Date.parse -> NaN -> effectiveMintKind returns the target kind).
expect(result.runOpsMintKindFlippedAt).toBe("not-a-date");
expect(
effectiveMintKind({ kind: "runOpsId", prev: "cuid", flippedAtMs: NaN }, T, GRACE_MS)
).toBe("runOpsId");
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});
77 changes: 77 additions & 0 deletions apps/webapp/app/v3/runOpsMigration/mintFlipGrace.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import type { RunIdMintKind } from "./runOpsMintKind.server";

export type { RunIdMintKind };

export type MintFlagResolution = {
kind: RunIdMintKind;
prev?: RunIdMintKind;
flippedAtMs?: number;
};

const DEFAULT_MINT_KIND: RunIdMintKind = "cuid";

// Deterministic wall-clock cutover: during [flippedAt, flippedAt + graceMs) every
// process — stale (hasn't re-read the flag) or fresh (has the new stamp) — resolves to
// the OLD kind. At/after the cutover, every process resolves to the NEW kind.
export function effectiveMintKind(
r: MintFlagResolution,
nowMs: number,
graceMs: number
): RunIdMintKind {
if (r.prev === undefined || r.flippedAtMs === undefined) {
return r.kind;
}
return nowMs < r.flippedAtMs + graceMs ? r.prev : r.kind;
}

function readMintKind(flags: Record<string, unknown>, key: string): RunIdMintKind | undefined {
const value = flags[key];
return value === "cuid" || value === "runOpsId" ? value : undefined;
}

function resolveEffectiveFromFlags(
flags: Record<string, unknown> | null | undefined,
nowMs: number,
graceMs: number
): RunIdMintKind {
const source = flags ?? {};
const kind = readMintKind(source, "runOpsMintKind") ?? DEFAULT_MINT_KIND;
const prev = readMintKind(source, "runOpsMintKindPrev");
const flippedAtRaw = source.runOpsMintKindFlippedAt;
const parsed = typeof flippedAtRaw === "string" ? Date.parse(flippedAtRaw) : NaN;
const flippedAtMs = Number.isNaN(parsed) ? undefined : parsed;

return effectiveMintKind({ kind, prev, flippedAtMs }, nowMs, graceMs);
}

// Stamps a grace window only when the outgoing TARGET kind differs from the stored one (a
// genuine flip); prev := the currently-effective kind. A save that leaves the target kind
// unchanged carries any in-flight stamp forward, so it can't reset the cutover clock.
export function stampMintKindFlip(
existingFlags: Record<string, unknown> | null | undefined,
outgoingFlags: Record<string, unknown>,
nowMs: number,
graceMs: number
): Record<string, unknown> {
const storedKind = readMintKind(existingFlags ?? {}, "runOpsMintKind") ?? DEFAULT_MINT_KIND;
const outgoingKind = readMintKind(outgoingFlags, "runOpsMintKind") ?? DEFAULT_MINT_KIND;
outgoingFlags.runOpsMintKind = outgoingKind;

if (outgoingKind !== storedKind) {
// Genuine target change: serve the currently-effective kind through the new grace window.
outgoingFlags.runOpsMintKindPrev = resolveEffectiveFromFlags(existingFlags, nowMs, graceMs);
outgoingFlags.runOpsMintKindFlippedAt = new Date(nowMs).toISOString();
return outgoingFlags;
}

const existing = existingFlags ?? {};
const existingPrev = existing.runOpsMintKindPrev;
const existingFlippedAt = existing.runOpsMintKindFlippedAt;
if (existingPrev !== undefined) {
outgoingFlags.runOpsMintKindPrev = existingPrev;
}
if (existingFlippedAt !== undefined) {
outgoingFlags.runOpsMintKindFlippedAt = existingFlippedAt;
}
return outgoingFlags;
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { BoundedTtlCache } from "~/services/realtime/boundedTtlCache";
import { computeRunIdMintKind, type RunIdMintKind } from "./runOpsMintKind.server";

// LOCK of the CURRENT (intentional) flip-latency behavior, NOT a change request.
// resolveRunIdMintKind caches the per-org mint kind in a process-singleton
// BoundedTtlCache (TTL RUN_OPS_MINT_FLAG_CACHE_TTL_MS, 30000ms default) with get/set
// and NO invalidation hook (runOpsMintKind.server.ts:38-45,56-81). So after a flag
// flip a process keeps minting the stale kind until its cached entry expires; in
// multi-instance prod each process expires independently. This suite reconstructs the
// same flag fn over a real cache and pins both edges of that window.
// LOCK of the raw per-process cache's flip-latency behavior in isolation, NOT a change
// request. Production resolveRunIdMintKind now wraps this same staleness in a deterministic
// wall-clock grace window (mintFlipGrace.ts) so every process resolves to the SAME effective
// kind for the whole window, then all cross together. This raw staleness is now an
// intentional, safe input to that resolution. computeRunIdMintKind is unaffected, so this
// suite's assertions stand as-is.

// Mirror of resolveRunIdMintKind's flag fn (runOpsMintKind.server.ts:56-81).
// Bare cached-flag closure — deliberately NOT the production flag fn, which now layers
// grace resolution on top (see runOpsMintKind.server.ts).
function makeCachedFlag(
cache: BoundedTtlCache<RunIdMintKind>,
liveFlag: () => RunIdMintKind
Expand Down
Loading