From cacab163c79a278be0c5498b7b02cae7132fde5b Mon Sep 17 00:00:00 2001 From: Noa Flaherty Date: Thu, 26 Feb 2026 22:15:46 -0500 Subject: [PATCH] fix: serialize feature-flag config writes to prevent concurrent races Co-Authored-By: Claude --- .../src/__tests__/feature-flags-route.test.ts | 37 ++++++++++ gateway/src/http/routes/feature-flags.ts | 72 +++++++++++-------- 2 files changed, 80 insertions(+), 29 deletions(-) diff --git a/gateway/src/__tests__/feature-flags-route.test.ts b/gateway/src/__tests__/feature-flags-route.test.ts index 6769576e91f..49227818778 100644 --- a/gateway/src/__tests__/feature-flags-route.test.ts +++ b/gateway/src/__tests__/feature-flags-route.test.ts @@ -471,4 +471,41 @@ describe("PATCH /v1/feature-flags/:flagKey handler", () => { const tmpFiles = files.filter((f: string) => f.endsWith(".tmp")); expect(tmpFiles.length).toBe(0); }); + + test("concurrent writes are serialized and no flag change is lost", async () => { + writeFileSync(configPath, JSON.stringify({})); + const handler = createFeatureFlagsPatchHandler(); + + // Fire multiple concurrent PATCH requests at the same time + const flagKeys = [ + "feature_flags.browser.enabled", + "feature_flags.twitter.enabled", + "feature_flags.guardian-verify-setup.enabled", + "feature_flags.hatch-new-assistant.enabled", + ]; + + const results = await Promise.all( + flagKeys.map((key) => + handler( + new Request(`http://gateway.test/v1/feature-flags/${key}`, { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ enabled: false }), + }), + key, + ), + ), + ); + + // All should succeed + for (const res of results) { + expect(res.status).toBe(200); + } + + // All flags should be persisted — none should be lost to a race + const config = JSON.parse(readFileSync(configPath, "utf-8")); + for (const key of flagKeys) { + expect(config.assistantFeatureFlagValues[key]).toBe(false); + } + }); }); diff --git a/gateway/src/http/routes/feature-flags.ts b/gateway/src/http/routes/feature-flags.ts index 8059a67e3fe..afdcbfd1016 100644 --- a/gateway/src/http/routes/feature-flags.ts +++ b/gateway/src/http/routes/feature-flags.ts @@ -7,6 +7,12 @@ import { getLogger } from "../../logger.js"; const log = getLogger("feature-flags"); +/** + * Serializes config writes so concurrent PATCH requests don't race on + * read-modify-write. Each write awaits the previous one before proceeding. + */ +let configWriteChain: Promise = Promise.resolve(); + /** * Only allow keys matching `feature_flags..enabled` for the canonical format. * The flagId segment must be a non-empty string of lowercase alphanumeric chars, @@ -202,34 +208,42 @@ export function createFeatureFlagsPatchHandler() { ); } - try { - const result = readConfigFile(); - if (!result.ok) { - log.error({ reason: result.reason, detail: result.detail }, "Config file is malformed, refusing to overwrite"); - return Response.json( - { error: "Config file is malformed, cannot safely write" }, - { status: 500 }, - ); - } - - const config = result.data; - - // Write to the new `assistantFeatureFlagValues` section (NOT the old `featureFlags` section) - const existingFlags = - config.assistantFeatureFlagValues && typeof config.assistantFeatureFlagValues === "object" && !Array.isArray(config.assistantFeatureFlagValues) - ? (config.assistantFeatureFlagValues as Record) - : {}; - - config.assistantFeatureFlagValues = { ...existingFlags, [flagKey]: enabled }; - - writeConfigFileAtomic(config); - - log.info({ flagKey, enabled }, "Feature flag updated"); - - return Response.json({ key: flagKey, enabled }); - } catch (err) { - log.error({ err, flagKey }, "Failed to update feature flag"); - return Response.json({ error: "Internal server error" }, { status: 500 }); - } + // Serialize config writes to prevent concurrent read-modify-write races + const writeResult = new Promise((resolve) => { + configWriteChain = configWriteChain.then(() => { + try { + const result = readConfigFile(); + if (!result.ok) { + log.error({ reason: result.reason, detail: result.detail }, "Config file is malformed, refusing to overwrite"); + resolve(Response.json( + { error: "Config file is malformed, cannot safely write" }, + { status: 500 }, + )); + return; + } + + const config = result.data; + + // Write to the new `assistantFeatureFlagValues` section (NOT the old `featureFlags` section) + const existingFlags = + config.assistantFeatureFlagValues && typeof config.assistantFeatureFlagValues === "object" && !Array.isArray(config.assistantFeatureFlagValues) + ? (config.assistantFeatureFlagValues as Record) + : {}; + + config.assistantFeatureFlagValues = { ...existingFlags, [flagKey]: enabled }; + + writeConfigFileAtomic(config); + + log.info({ flagKey, enabled }, "Feature flag updated"); + + resolve(Response.json({ key: flagKey, enabled })); + } catch (err) { + log.error({ err, flagKey }, "Failed to update feature flag"); + resolve(Response.json({ error: "Internal server error" }, { status: 500 })); + } + }); + }); + + return writeResult; }; }