Skip to content
Merged
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
37 changes: 37 additions & 0 deletions gateway/src/__tests__/feature-flags-route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});
});
72 changes: 43 additions & 29 deletions gateway/src/http/routes/feature-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> = Promise.resolve();

/**
* Only allow keys matching `feature_flags.<flagId>.enabled` for the canonical format.
* The flagId segment must be a non-empty string of lowercase alphanumeric chars,
Expand Down Expand Up @@ -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<string, unknown>)
: {};

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<Response>((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<string, unknown>)
: {};

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;
};
}
Loading